[Pkg-owncloud-commits] [owncloud-client] 312/332: SyncEngine: Keep csync_journal with proper values for fileId and remotePerm
Sandro Knauß
hefee-guest at moszumanska.debian.org
Thu Aug 14 21:07:16 UTC 2014
This is an automated email from the git hooks/post-receive script.
hefee-guest pushed a commit to branch master
in repository owncloud-client.
commit 22c1629dd318ad150d7004d7da3e370b544c33d1
Author: Markus Goetz <markus at woboq.com>
Date: Thu Aug 7 10:14:14 2014 +0200
SyncEngine: Keep csync_journal with proper values for fileId and remotePerm
Before this patch, we had a lot of empty rows because we created
the SyncFileItems with the wrong(=local) data.
---
csync/src/csync_update.c | 1 +
csync/tests/ownCloud/t7.pl | 22 +++++++++++++++
src/mirall/syncengine.cpp | 52 +++++++++++++++++++++++++++++-------
src/mirall/syncengine.h | 2 ++
src/mirall/syncjournaldb.cpp | 13 ++++++---
src/mirall/syncjournalfilerecord.cpp | 1 +
6 files changed, 77 insertions(+), 14 deletions(-)
diff --git a/csync/src/csync_update.c b/csync/src/csync_update.c
index 2fab730..36270e3 100644
--- a/csync/src/csync_update.c
+++ b/csync/src/csync_update.c
@@ -269,6 +269,7 @@ static int _csync_detect_update(CSYNC *ctx, const char *file,
}
if (metadata_differ) {
/* file id or permissions has changed. Which means we need to update them in the DB. */
+ CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "Need to update metadata for: %s", path);
st->should_update_etag = true;
}
st->instruction = CSYNC_INSTRUCTION_NONE;
diff --git a/csync/tests/ownCloud/t7.pl b/csync/tests/ownCloud/t7.pl
index 837c4da..3288d66 100755
--- a/csync/tests/ownCloud/t7.pl
+++ b/csync/tests/ownCloud/t7.pl
@@ -31,6 +31,14 @@ use strict;
print "Hello, this is t7, a tester for syncing of files in read only directory\n";
+# Check if the expected rows in the DB are non-empty. Note that in some cases they might be, then we cannot use this function
+# https://github.com/owncloud/mirall/issues/2038
+sub assertCsyncJournalOk {
+ my $path = $_[0];
+ my $cmd = 'sqlite3 ' . $path . '.csync_journal.db "SELECT count(*) from metadata where length(remotePerm) == 0 or length(fileId) == 0 or length(md5) == 0"';
+ my $result = `$cmd`;
+ assert($result == "0");
+}
# IMPORTANT NOTE :
print "This test use the OWNCLOUD_TEST_PERMISSIONS environement variable and _PERM_xxx_ on filenames to set the permission. ";
@@ -63,6 +71,7 @@ glob_put( "$tmpdir/normalFile_PERM_WVND_.data", "readonlyDirectory_PERM_M_/subdi
csync();
+assertCsyncJournalOk(localDir());
assertLocalAndRemoteDir( '', 0);
system("sleep 1"); #make sure changes have different mtime
@@ -99,6 +108,7 @@ createLocalFile( localDir() . "normalDirectory_PERM_CKDNV_/newFile_PERM_WDNV_.da
#do the sync
csync();
+assertCsyncJournalOk(localDir());
#1.
@@ -149,6 +159,7 @@ printInfo( "remove the read only directory" );
# -> It must be recovered
system("rm -r " . localDir().'readonlyDirectory_PERM_M_' );
csync();
+assertCsyncJournalOk(localDir());
assert( -e localDir(). 'readonlyDirectory_PERM_M_/cannotBeRemoved_PERM_WVN_.data' );
assert( -e localDir(). 'readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data' );
assertLocalAndRemoteDir( '', 0);
@@ -156,15 +167,23 @@ assertLocalAndRemoteDir( '', 0);
#######################################################################
printInfo( "move a directory in a outside read only folder" );
+system("sqlite3 " . localDir().'.csync_journal.db .dump');
+
#Missing directory should be restored
#new directory should be uploaded
system("mv " . localDir().'readonlyDirectory_PERM_M_/subdir_PERM_CK_ ' . localDir().'normalDirectory_PERM_CKDNV_/subdir_PERM_CKDNV_' );
# two syncs may be necessary for now
csync();
+system("sqlite3 " . localDir().'.csync_journal.db .dump');
+printInfo("CRAAAAAAAP");
+#assertCsyncJournalOk(localDir()); <-- some fileId entries are empty so we don't check the DB here. See https://github.com/owncloud/mirall/issues/2038
csync();
+system("sqlite3 " . localDir().'.csync_journal.db .dump');
+assertCsyncJournalOk(localDir());
# old name restored
+assert( -e localDir(). 'readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/' );
assert( -e localDir(). 'readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data' );
# new still exist
@@ -192,7 +211,9 @@ system("mv " . localDir().'normalDirectory_PERM_CKDNV_/subdir_PERM_CKDNV_ ' . lo
# two syncs may be necessary for now
csync();
+assertCsyncJournalOk(localDir());
csync();
+assertCsyncJournalOk(localDir());
#1.
# old name restored
@@ -213,6 +234,7 @@ system("rm -r " . localDir(). "readonlyDirectory_PERM_M_/moved_PERM_CK_");
assertLocalAndRemoteDir( '', 0);
+system("sqlite3 " . localDir().'.csync_journal.db .dump');
cleanup();
diff --git a/src/mirall/syncengine.cpp b/src/mirall/syncengine.cpp
index 3370811..c68be74 100644
--- a/src/mirall/syncengine.cpp
+++ b/src/mirall/syncengine.cpp
@@ -21,6 +21,7 @@
#include "syncjournaldb.h"
#include "syncjournalfilerecord.h"
#include "creds/abstractcredentials.h"
+#include "csync_util.h"
#ifdef Q_OS_WIN
#include <windows.h>
@@ -262,12 +263,26 @@ int SyncEngine::treewalkRemote( TREE_WALK_FILE* file, void *data )
int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote )
{
if( ! file ) return -1;
- SyncFileItem item;
- item._file = QString::fromUtf8( file->path );
+
+ QString fileUtf8 = QString::fromUtf8( file->path );
+
+ // Gets a default-contructed SyncFileItem or the one from the first walk (=local walk)
+ SyncFileItem item = _syncItemMap.value(fileUtf8);
+ item._file = fileUtf8;
item._originalFile = item._file;
- item._instruction = file->instruction;
- item._direction = SyncFileItem::None;
- item._fileId = file->file_id;
+
+ if (item._instruction == CSYNC_INSTRUCTION_NONE) {
+ item._instruction = file->instruction;
+ item._modtime = file->modtime;
+ } else {
+ if (file->instruction != CSYNC_INSTRUCTION_NONE) {
+ Q_ASSERT(!"Instructions are both unequal NONE");
+ }
+ }
+
+ if (file->file_id && strlen(file->file_id) > 0) {
+ item._fileId = file->file_id;
+ }
if (file->directDownloadUrl) {
item._directDownloadUrl = QString::fromUtf8( file->directDownloadUrl );
}
@@ -277,6 +292,7 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote )
if (file->remotePerm && file->remotePerm[0]) {
item._remotePerm = QByteArray(file->remotePerm);
}
+ item._should_update_etag = item._should_update_etag || file->should_update_etag;
// record the seen files to be able to clean the journal later
_seenFiles.insert(item._file);
@@ -306,12 +322,16 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote )
/* No error string */
}
item._isDirectory = file->type == CSYNC_FTW_TYPE_DIR;
- item._modtime = file->modtime;
+
+ // The etag is already set in the previous sync phases somewhere. Maybe we should remove it there
+ // and do it here so we have a consistent state about which tree stores information from which source.
item._etag = file->etag;
item._size = file->size;
- item._inode = file->inode;
- item._should_update_etag = file->should_update_etag;
+ if (!remote) {
+ item._inode = file->inode;
+ }
+
switch( file->type ) {
case CSYNC_FTW_TYPE_DIR:
item._type = SyncFileItem::Directory;
@@ -329,11 +349,15 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote )
SyncFileItem::Direction dir;
int re = 0;
-
switch(file->instruction) {
case CSYNC_INSTRUCTION_NONE:
if (file->should_update_etag && !item._isDirectory) {
// Update the database now already (new fileid or etag or remotePerm)
+ // Those are files that were detected as "resolved conflict".
+ // They should have been a conflict because they both were new, or both
+ // had their local mtime or remote etag modified, but the size and mtime
+ // is the same on the server. This typically happen when the database is removed.
+ // Nothing will be done for those file, but we still need to update the database.
_journal->setFileRecord(SyncJournalFileRecord(item, _localPath + item._file));
item._should_update_etag = false;
}
@@ -402,7 +426,7 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote )
item.log._other_modtime = file->other.modtime;
item.log._other_size = file->other.size;
- _syncedItems.append(item);
+ _syncItemMap.insert(fileUtf8, item);
emit syncItemDiscovered(item);
return re;
@@ -446,6 +470,7 @@ void SyncEngine::startSync()
}
_syncedItems.clear();
+ _syncItemMap.clear();
_needsUpdate = false;
csync_resume(_csync_ctx);
@@ -553,6 +578,9 @@ void SyncEngine::slotUpdateFinished(int updateResult)
qDebug() << "Error in remote treewalk.";
}
+ // The map was used for merging trees, convert it to a list:
+ _syncedItems = _syncItemMap.values().toVector();
+
// Adjust the paths for the renames.
for (SyncFileItemVector::iterator it = _syncedItems.begin();
it != _syncedItems.end(); ++it) {
@@ -762,10 +790,12 @@ void SyncEngine::checkForPermission()
break;
} if (!it->_isDirectory && !perms.contains("W")) {
qDebug() << "checkForPermission: RESTORING" << it->_file;
+ it->_should_update_etag = true;
it->_instruction = CSYNC_INSTRUCTION_CONFLICT;
it->_direction = SyncFileItem::Down;
it->_isRestoration = true;
// take the things to write to the db from the "other" node (i.e: info from server)
+ // ^^ FIXME This might not be needed anymore since we merge the info in treewalkFile
it->_modtime = it->log._other_modtime;
it->_fileId = it->log._other_fileId;
it->_etag = it->log._other_etag;
@@ -781,6 +811,7 @@ void SyncEngine::checkForPermission()
break;
} if (!perms.contains("D")) {
qDebug() << "checkForPermission: RESTORING" << it->_file;
+ it->_should_update_etag = true;
it->_instruction = CSYNC_INSTRUCTION_NEW;
it->_direction = SyncFileItem::Down;
it->_isRestoration = true;
@@ -800,6 +831,7 @@ void SyncEngine::checkForPermission()
}
qDebug() << "checkForPermission: RESTORING" << it->_file;
+ it->_should_update_etag = true;
it->_instruction = CSYNC_INSTRUCTION_NEW;
it->_direction = SyncFileItem::Down;
diff --git a/src/mirall/syncengine.h b/src/mirall/syncengine.h
index 34d919a..c939c33 100644
--- a/src/mirall/syncengine.h
+++ b/src/mirall/syncengine.h
@@ -22,6 +22,7 @@
#include <QThread>
#include <QString>
#include <QSet>
+#include <QMap>
#include <qelapsedtimer.h>
#include <csync.h>
@@ -106,6 +107,7 @@ private:
void finalize();
static bool _syncRunning; //true when one sync is running somewhere (for debugging)
+ QMap<QString, SyncFileItem> _syncItemMap;
SyncFileItemVector _syncedItems;
CSYNC *_csync_ctx;
diff --git a/src/mirall/syncjournaldb.cpp b/src/mirall/syncjournaldb.cpp
index 63b00e7..c1f1aa8 100644
--- a/src/mirall/syncjournaldb.cpp
+++ b/src/mirall/syncjournaldb.cpp
@@ -833,10 +833,15 @@ void SyncJournalDb::avoidRenamesOnNextSync(const QString& path)
query.bindValue(0, path);
query.bindValue(1, path);
if( !query.exec() ) {
- qDebug() << "SQL error in avoidRenamesOnNextSync: "<< query.lastError().text();
+ qDebug() << Q_FUNC_INFO << "SQL error in avoidRenamesOnNextSync: "<< query.lastError().text();
} else {
- qDebug() << query.executedQuery() << path;
+ qDebug() << Q_FUNC_INFO << query.executedQuery() << path << "(" << query.numRowsAffected() << " rows)";
}
+
+ // We also need to remove the ETags so the update phase refreshes the directory paths
+ // on the next sync
+ locker.unlock();
+ avoidReadFromDbOnNextSync(path);
}
void SyncJournalDb::avoidReadFromDbOnNextSync(const QString& fileName)
@@ -856,9 +861,9 @@ void SyncJournalDb::avoidReadFromDbOnNextSync(const QString& fileName)
query.prepare("UPDATE metadata SET md5='_invalid_' WHERE ? LIKE(path||'/%') AND type == 2"); // CSYNC_FTW_TYPE_DIR == 2
query.bindValue(0, fileName);
if( !query.exec() ) {
- qDebug() << "SQL error in avoidRenamesOnNextSync: "<< query.lastError().text();
+ qDebug() << Q_FUNC_INFO << "SQL error in avoidRenamesOnNextSync: "<< query.lastError().text();
} else {
- qDebug() << query.executedQuery() << fileName;
+ qDebug() << Q_FUNC_INFO << query.executedQuery() << fileName << "(" << query.numRowsAffected() << " rows)";
}
}
diff --git a/src/mirall/syncjournalfilerecord.cpp b/src/mirall/syncjournalfilerecord.cpp
index f29ec88..dbc495d 100644
--- a/src/mirall/syncjournalfilerecord.cpp
+++ b/src/mirall/syncjournalfilerecord.cpp
@@ -77,6 +77,7 @@ SyncJournalFileRecord::SyncJournalFileRecord(const SyncFileItem &item, const QSt
_inode = sb.st_ino;
}
#endif
+ qDebug() << Q_FUNC_INFO << localFileName << "Retrieved inode " << _inode << "(previous item inode: " << item._inode << ")";
}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud-client.git
More information about the Pkg-owncloud-commits
mailing list