Add blacklisting for files with error conditions.

8 files changed, 233 insertions, 5 deletions

diff --git a/src/mirall/csyncthread.cpp b/src/mirall/csyncthread.cpp
index 607019807..b161b82f0 100644
--- a/src/mirall/csyncthread.cpp
+++ b/src/mirall/csyncthread.cpp
@@ -184,6 +184,53 @@ QString CSyncThread::csyncErrorToString(CSYNC_STATUS err)
 
 }
 
+bool CSyncThread::checkBlacklisting( SyncFileItem *item )
+{
+    bool re = false;
+
+    if( !_journal ) {
+        qWarning() << "Journal is undefined!";
+        return false;
+    }
+
+    SyncJournalBlacklistRecord entry = _journal->blacklistEntry(item->_file);
+    item->_blacklistedInDb = false;
+
+    // if there is a valid entry in the blacklist table and the retry count is
+    // already null or smaller than 0, the file is blacklisted.
+    if( entry.isValid() ) {
+        if( entry._retryCount <= 0 ) {
+            re = true;
+            item->_blacklistedInDb = true;
+        }
+
+        // if the retryCount is 0, but the etag has changed, it is tried again
+        // note that if the retryCount is -1 we never try again.
+        if( entry._retryCount == 0 ) {
+            if( item->_etag.isEmpty() || entry._lastTryEtag.isEmpty() ) {
+                // compare the mtimes.
+                if(entry._lastTryModtime != item->_modtime) {
+                    re = false;
+                    qDebug() << item->_file << " is blacklisted, but has changed mtime!";
+
+                }
+            } else {
+                if( entry._lastTryEtag != item->_etag) {
+                    re = false;
+                    qDebug() << item->_file << " is blacklisted, but has changed etag!";
+                }
+            }
+        }
+        if( re ) {
+            qDebug() << "Item is on blacklist: " << entry._file << "retries:" << entry._retryCount;
+            item->_blacklistedInDb = true;
+            item->_instruction = CSYNC_INSTRUCTION_IGNORE;
+        }
+    }
+
+    return re;
+}
+
 int CSyncThread::treewalkLocal( TREE_WALK_FILE* file, void *data )
 {
     return static_cast<CSyncThread*>(data)->treewalkFile( file, false );
@@ -204,6 +251,7 @@ int CSyncThread::treewalkFile( TREE_WALK_FILE *file, bool remote )
     item._dir = SyncFileItem::None;
     item._fileId = QString::fromUtf8(file->file_id);
 
+    // record the seen files to be able to clean the journal later
     _seenFiles[item._file] = QString();
 
     if(file->error_string) {
@@ -220,6 +268,10 @@ int CSyncThread::treewalkFile( TREE_WALK_FILE *file, bool remote )
 
     int re = 0;
 
+    // check for blacklisting of this item.
+    // if the item is on blacklist, the instruction was set to IGNORE
+    checkBlacklisting( &item );
+
     if (file->instruction != CSYNC_INSTRUCTION_IGNORE
         && file->instruction != CSYNC_INSTRUCTION_REMOVE) {
       _hasFiles = true;
diff --git a/src/mirall/csyncthread.h b/src/mirall/csyncthread.h
index ff81740c0..4cbe1eb6f 100644
--- a/src/mirall/csyncthread.h
+++ b/src/mirall/csyncthread.h
@@ -87,6 +87,7 @@ private:
     static int treewalkLocal( TREE_WALK_FILE*, void *);
     static int treewalkRemote( TREE_WALK_FILE*, void *);
     int treewalkFile( TREE_WALK_FILE*, bool );
+    bool checkBlacklisting( SyncFileItem *item );
 
     static QMutex _mutex;
     static QMutex _syncMutex;
diff --git a/src/mirall/owncloudpropagator.cpp b/src/mirall/owncloudpropagator.cpp
index 31edd5bdb..f773bdbba 100644
--- a/src/mirall/owncloudpropagator.cpp
+++ b/src/mirall/owncloudpropagator.cpp
@@ -62,10 +62,49 @@ class NAME : public PropagateItemJob { \
   /*  Q_OBJECT */ \
 public: \
     NAME(OwncloudPropagator* propagator,const SyncFileItem& item) \
-        : PropagateItemJob(propagator, item) {} \
+    : PropagateItemJob(propagator, item) {} \
     void start(); \
 };
 
+void PropagateItemJob::done(SyncFileItem::Status status, const QString &errorString)
+{
+    _item._errorString = errorString;
+    _item._status = status;
+
+    // Blacklisting
+    int retries = 0;
+
+    if( _item._httpErrorCode == 403 || _item._httpErrorCode == 413 || _item._httpErrorCode == 415 ) {
+        qDebug() << "Fatal Error condition, disallow retry!";
+        retries = -1;
+    } else {
+        retries = 3; // FIXME: good number of allowed retries?
+    }
+    SyncJournalBlacklistRecord record(_item, retries);;
+
+    switch( status ) {
+    case SyncFileItem::FatalError:
+    case SyncFileItem::NormalError:
+    case SyncFileItem::SoftError:
+        _propagator->_journal->updateBlacklistEntry( record );
+        break;
+    case SyncFileItem::Success:
+        if( _item._blacklistedInDb ) {
+            // wipe blacklist entry.
+            _propagator->_journal->wipeBlacklistEntry(_item._file);
+        }
+        break;
+    case SyncFileItem::Conflict:
+    case SyncFileItem::FileIgnored:
+    case SyncFileItem::NoStatus:
+        // nothing
+        break;
+    }
+
+    emit completed(_item);
+    emit finished(status);
+}
+
 // compare two files with given filename and return true if they have the same content
 static bool fileEquals(const QString &fn1, const QString &fn2) {
     QFile f1(fn1);
diff --git a/src/mirall/syncfileitem.h b/src/mirall/syncfileitem.h
index 4fbec1602..e0a9da7fa 100644
--- a/src/mirall/syncfileitem.h
+++ b/src/mirall/syncfileitem.h
@@ -82,6 +82,7 @@ public:
     QByteArray           _etag;
     quint64              _size;
     bool                 _should_update_etag;
+    bool                _blacklistedInDb;
 
     // Variables usefull to report to the user
     Status              _status;
diff --git a/src/mirall/syncjournaldb.cpp b/src/mirall/syncjournaldb.cpp
index ea4061832..7c9669867 100644
--- a/src/mirall/syncjournaldb.cpp
+++ b/src/mirall/syncjournaldb.cpp
@@ -147,6 +147,21 @@ bool SyncJournalDb::checkConnect()
         return false;
     }
 
+    // create the blacklist table.
+    createQuery.prepare("CREATE TABLE IF NOT EXISTS blacklist ("
+                        "path VARCHAR(4096),"
+                        "lastTryEtag VARCHAR[32],"
+                        "lastTryModtime INTEGER[8],"
+                        "retrycount INTEGER default 0,"
+                        "errorstring VARCHAR[4096],"
+                        "PRIMARY KEY(path)"
+                        ");");
+
+    if (!createQuery.exec()) {
+        qWarning() << "Error creating table blacklist: " << createQuery.lastError().text();
+        return false;
+    }
+
     bool rc = updateDatabaseStructure();
     if( rc ) {
         _getFileRecordQuery.reset(new QSqlQuery(_db));
@@ -189,6 +204,9 @@ bool SyncJournalDb::checkConnect()
         _deleteFileRecordRecursively.reset(new QSqlQuery(_db));
         _deleteFileRecordRecursively->prepare("DELETE FROM metadata WHERE path LIKE(?||'/%')");
 
+        _blacklistQuery.reset(new QSqlQuery(_db));
+        _blacklistQuery->prepare("SELECT lastTryEtag, lastTryModtime, retrycount, errorstring "
+                                 "FROM blacklist WHERE path=:path");
     }
     return rc;
 }
@@ -207,6 +225,7 @@ void SyncJournalDb::close()
     _deleteUploadInfoQuery.reset(0);
     _deleteFileRecordPhash.reset(0);
     _deleteFileRecordRecursively.reset(0);
+    _blacklistQuery.reset(0);
     _db.close();
 }
 
@@ -214,17 +233,19 @@ void SyncJournalDb::close()
 bool SyncJournalDb::updateDatabaseStructure()
 {
     QStringList columns = tableColumns("metadata");
+    bool re = true;
 
     // check if the file_id column is there and create it if not
     if( columns.indexOf(QLatin1String("fileid")) == -1 ) {
         QSqlQuery query(_db);
         query.prepare("ALTER TABLE metadata ADD COLUMN fileid VARCHAR(128);");
-        query.exec();
+        re = query.exec();
 
         query.prepare("CREATE INDEX metadata_file_id ON metadata(fileid);");
-        query.exec();
+        re = re && query.exec();
     }
-    return true;
+
+    return re;
 }
 
 QStringList SyncJournalDb::tableColumns( const QString& table )
@@ -581,6 +602,91 @@ void SyncJournalDb::setUploadInfo(const QString& file, const SyncJournalDb::Uplo
     }
 }
 
+SyncJournalBlacklistRecord SyncJournalDb::blacklistEntry( const QString& file )
+{
+    QMutexLocker locker(&_mutex);
+    SyncJournalBlacklistRecord entry;
+
+    if( file.isEmpty() ) return entry;
+
+    // SELECT lastTryEtag, lastTryModtime, retrycount, errorstring
+
+    if( checkConnect() ) {
+        _blacklistQuery->bindValue( ":path", file );
+        if( _blacklistQuery->exec() ){
+            if( _blacklistQuery->next() ) {
+                bool ok;
+                entry._lastTryEtag    = _blacklistQuery->value(0).toByteArray();
+                entry._lastTryModtime = _blacklistQuery->value(1).toLongLong(&ok);
+                entry._retryCount     = _blacklistQuery->value(2).toInt();
+                entry._errorString    = _blacklistQuery->value(3).toString();
+                entry._file           = file;
+            }
+        } else {
+            qWarning() << "Exec error blacklist: " << _blacklistQuery->lastQuery() <<  " : "
+                       << _blacklistQuery->lastError().text();
+        }
+        _blacklistQuery->finish();
+    }
+
+    return entry;
+}
+
+void SyncJournalDb::wipeBlacklistEntry( const QString& file )
+{
+    QMutexLocker locker(&_mutex);
+
+    QSqlQuery query;
+
+    query.prepare("DELETE FROM blacklist WHERE path=:path");
+    query.bindValue(":path", file);
+    if( ! query.exec() ) {
+        qDebug() << "Deletion of blacklist item failed.";
+    }
+}
+
+void SyncJournalDb::updateBlacklistEntry( const SyncJournalBlacklistRecord& item )
+{
+    QMutexLocker locker(&_mutex);
+    QSqlQuery query;
+
+    query.prepare("SELECT retrycount FROM blacklist WHERE path=:path");
+    query.bindValue(":path", item._file);
+
+    if( !query.exec() ) {
+        qDebug() << "SQL exec blacklistitem failed.";
+        return;
+    }
+
+    QSqlQuery iQuery;
+    if( query.next() ) {
+        int retries = query.value(0).toInt();
+        retries--;
+        if( retries < 0 ) retries = 0;
+
+        iQuery.prepare( "UPDATE blacklist SET lastTryEtag = :etag, lastTryModtime = :modtime, "
+                        "retrycount = :retries, errorstring = :errStr WHERE path=:path");
+        iQuery.bindValue(":etag", item._lastTryEtag);
+        iQuery.bindValue(":modtime", QString::number(item._lastTryModtime));
+        iQuery.bindValue(":retries", retries);
+        iQuery.bindValue(":errStr", item._errorString);
+        iQuery.bindValue(":path", item._file);
+    } else {
+        // there is no entry yet.
+        iQuery.prepare("INSERT INTO blacklist (path, lastTryEtag, lastTryModtime, retrycount, errorstring) "
+                         "VALUES (:path, :lastEtag, :lastMTime, :retrycount, :errorstring);");
+
+        iQuery.bindValue(":path", item._file );
+        iQuery.bindValue(":lastEtag", item._lastTryEtag);
+        iQuery.bindValue(":lastMTime", QString::number(item._lastTryModtime));
+        iQuery.bindValue(":retrycount", item._retryCount);
+        iQuery.bindValue(":errorstring", item._errorString);
+    }
+    if( !iQuery.exec() ) {
+        qDebug() << "SQL exec blacklistitem insert/update failed: "<< iQuery.lastError().text();
+    }
+}
+
 void SyncJournalDb::commit()
 {
     QMutexLocker locker(&_mutex);
diff --git a/src/mirall/syncjournaldb.h b/src/mirall/syncjournaldb.h
index d3a2dff32..778c84f18 100644
--- a/src/mirall/syncjournaldb.h
+++ b/src/mirall/syncjournaldb.h
@@ -23,6 +23,7 @@
 
 namespace Mirall {
 class SyncJournalFileRecord;
+class SyncJournalBlacklistRecord;
 
 class SyncJournalDb : public QObject
 {
@@ -36,6 +37,8 @@ public:
     int getFileRecordCount();
     bool exists();
     QStringList tableColumns( const QString& table );
+    void updateBlacklistEntry( const SyncJournalBlacklistRecord& item );
+    void wipeBlacklistEntry(const QString& file);
 
     struct DownloadInfo {
         DownloadInfo() : _errorCount(0), _valid(false) {}
@@ -58,6 +61,8 @@ public:
     void setDownloadInfo(const QString &file, const DownloadInfo &i);
     UploadInfo getUploadInfo(const QString &file);
     void setUploadInfo(const QString &file, const UploadInfo &i);
+    SyncJournalBlacklistRecord blacklistEntry( const QString& );
+
     bool postSyncCleanup( const QHash<QString, QString>& items );
 
     /* Because sqlite transactions is really slow, we encapsulate everything in big transactions
@@ -88,7 +93,7 @@ private:
     QScopedPointer<QSqlQuery> _deleteUploadInfoQuery;
     QScopedPointer<QSqlQuery> _deleteFileRecordPhash;
     QScopedPointer<QSqlQuery> _deleteFileRecordRecursively;
-
+    QScopedPointer<QSqlQuery> _blacklistQuery;
 };
 
 }  // namespace Mirall
diff --git a/src/mirall/syncjournalfilerecord.cpp b/src/mirall/syncjournalfilerecord.cpp
index f64b601e8..8607400cf 100644
--- a/src/mirall/syncjournalfilerecord.cpp
+++ b/src/mirall/syncjournalfilerecord.cpp
@@ -83,4 +83,11 @@ SyncJournalFileRecord::SyncJournalFileRecord(const SyncFileItem &item, const QSt
 
 }
 
+SyncJournalBlacklistRecord::SyncJournalBlacklistRecord(const SyncFileItem& item, int retries)
+    :_retryCount(retries), _errorString(item._errorString), _lastTryModtime(item._modtime)
+    , _lastTryEtag(item._etag), _file(item._file)
+{
+
+}
+
 }
diff --git a/src/mirall/syncjournalfilerecord.h b/src/mirall/syncjournalfilerecord.h
index 0625d8d39..669ca1173 100644
--- a/src/mirall/syncjournalfilerecord.h
+++ b/src/mirall/syncjournalfilerecord.h
@@ -44,6 +44,23 @@ public:
     int       _mode;
 };
 
+class SyncJournalBlacklistRecord
+{
+public:
+    SyncJournalBlacklistRecord() : _retryCount(0), _lastTryModtime(0) { }
+
+    SyncJournalBlacklistRecord(const SyncFileItem&, int retries);
+
+    // query("SELECT path, inode, uid, gid, mode, modtime, type, md5 FROM metadata WHERE phash=:phash");
+    int        _retryCount;
+    QString    _errorString;
+    time_t     _lastTryModtime;
+    QByteArray _lastTryEtag;
+    QString    _file;
+
+    bool isValid() { return(_lastTryEtag.length() > 0 || _lastTryModtime > 0); }
+};
+
 }
 
 #endif // SYNCJOURNALFILERECORD_H