From 17dd199cbac63ad44a585d1a3095a6fa6fde756a Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 10 Nov 2015 15:05:00 +0100 Subject: [PATCH] Checksums: Treat more carefully in db #4034 In particular, preserve them on local rename or remote move. --- src/libsync/owncloudpropagator.cpp | 2 +- src/libsync/propagateremotemove.cpp | 5 +++ src/libsync/propagatorjobs.cpp | 4 ++ src/libsync/syncengine.cpp | 2 +- src/libsync/syncjournaldb.cpp | 64 ++++++++--------------------- src/libsync/syncjournaldb.h | 6 ++- test/testsyncjournaldb.h | 2 +- 7 files changed, 32 insertions(+), 53 deletions(-) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index bd4f37b62..478f5fa5b 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -634,7 +634,7 @@ void PropagateDirectory::finalize() } } SyncJournalFileRecord record(*_item, _propagator->_localDir + _item->_file); - _propagator->_journal->setFileRecord(record); + _propagator->_journal->setFileRecordMetadata(record); } } _state = Finished; diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index 64b85f81c..33501aa49 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -152,9 +152,14 @@ void PropagateRemoteMove::slotMoveJobFinished() void PropagateRemoteMove::finalize() { + SyncJournalFileRecord oldRecord = + _propagator->_journal->getFileRecord(_item->_originalFile); _propagator->_journal->deleteFileRecord(_item->_originalFile); + SyncJournalFileRecord record(*_item, _propagator->getFilePath(_item->_renameTarget)); record._path = _item->_renameTarget; + record._transmissionChecksum = oldRecord._transmissionChecksum; + record._transmissionChecksumType = oldRecord._transmissionChecksumType; _propagator->_journal->setFileRecord(record); _propagator->_journal->commit("Remote Rename"); diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 29acc1cb3..ddb17ef37 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -200,6 +200,8 @@ void PropagateLocalRename::start() } } + SyncJournalFileRecord oldRecord = + _propagator->_journal->getFileRecord(_item->_originalFile); _propagator->_journal->deleteFileRecord(_item->_originalFile); // store the rename file name in the item. @@ -207,6 +209,8 @@ void PropagateLocalRename::start() SyncJournalFileRecord record(*_item, targetFile); record._path = _item->_renameTarget; + record._transmissionChecksum = oldRecord._transmissionChecksum; + record._transmissionChecksumType = oldRecord._transmissionChecksumType; if (!_item->_isDirectory) { // Directories are saved at the end _propagator->_journal->setFileRecord(record); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 54ced86ce..94fe02716 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -484,7 +484,7 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote ) FileSystem::setFileReadOnly(filePath, isReadOnly); } - _journal->updateFileRecordMetadata(SyncJournalFileRecord(*item, filePath)); + _journal->setFileRecordMetadata(SyncJournalFileRecord(*item, filePath)); item->_should_update_metadata = false; } if (item->_isDirectory && file->should_update_metadata) { diff --git a/src/libsync/syncjournaldb.cpp b/src/libsync/syncjournaldb.cpp index 75b55d9b9..b5f8fff2c 100644 --- a/src/libsync/syncjournaldb.cpp +++ b/src/libsync/syncjournaldb.cpp @@ -374,13 +374,6 @@ bool SyncJournalDb::checkConnect() " SET transmissionChecksum = ?2, transmissionChecksumTypeId = ?3" " WHERE phash == ?1;"); - _setFileRecordMetadataQuery.reset(new SqlQuery(_db) ); - _setFileRecordMetadataQuery->prepare( - "UPDATE metadata" - " SET inode=?2, mode=?3, modtime=?4, type=?5, md5=?6, fileid=?7," - " remotePerm=?8, filesize=?9, ignoredChildrenRemote=?10" - " WHERE phash == ?1;"); - _getDownloadInfoQuery.reset(new SqlQuery(_db) ); _getDownloadInfoQuery->prepare( "SELECT tmpfile, etag, errorcount FROM " "downloadinfo WHERE path=?1" ); @@ -458,7 +451,6 @@ void SyncJournalDb::close() _getFileRecordQuery.reset(0); _setFileRecordQuery.reset(0); _setFileRecordChecksumQuery.reset(0); - _setFileRecordMetadataQuery.reset(0); _getDownloadInfoQuery.reset(0); _setDownloadInfoQuery.reset(0); _deleteDownloadInfoQuery.reset(0); @@ -920,50 +912,26 @@ bool SyncJournalDb::updateFileRecordChecksum(const QString& filename, return true; } -bool SyncJournalDb::updateFileRecordMetadata(const SyncJournalFileRecord& record) +bool SyncJournalDb::setFileRecordMetadata(const SyncJournalFileRecord& record) { - QMutexLocker locker(&_mutex); + SyncJournalFileRecord existing = getFileRecord(record._path); - qlonglong phash = getPHash(record._path); - QString etag( record._etag ); - if( etag.isEmpty() ) etag = ""; - QString fileId( record._fileId); - if( fileId.isEmpty() ) fileId = ""; - QString remotePerm (record._remotePerm); - if (remotePerm.isEmpty()) remotePerm = QString(); // have NULL in DB (vs empty) - - if( !checkConnect() ) { - qDebug() << "Failed to connect database."; - return false; + // If there's no existing record, just insert the new one. + if (existing._path.isEmpty()) { + return setFileRecord(record); } - auto & query = _setFileRecordMetadataQuery; - - query->reset(); - query->bindValue(1, QString::number(phash)); - query->bindValue(2, record._inode); - query->bindValue(3, record._mode); - query->bindValue(4, QString::number(Utility::qDateTimeToTime_t(record._modtime))); - query->bindValue(5, QString::number(record._type)); - query->bindValue(6, etag); - query->bindValue(7, fileId); - query->bindValue(8, remotePerm); - query->bindValue(9, record._fileSize); - query->bindValue(10, record._serverHasIgnoredFiles ? 1 : 0); - - if( !query->exec() ) { - qWarning() << "Error SQL statement setFileRecordMetadataQuery: " - << query->lastQuery() << " :" - << query->error(); - return false; - } - - qDebug() << query->lastQuery() << record._path << record._inode << record._mode << record._modtime - << record._type << etag << fileId << remotePerm << record._fileSize - << record._serverHasIgnoredFiles; - - query->reset(); - return true; + // Update the metadata on the existing record. + existing._inode = record._inode; + existing._mode = record._mode; + existing._modtime = record._modtime; + existing._type = record._type; + existing._etag = record._etag; + existing._fileId = record._fileId; + existing._remotePerm = record._remotePerm; + existing._fileSize = record._fileSize; + existing._serverHasIgnoredFiles = record._serverHasIgnoredFiles; + return setFileRecord(existing); } static void toDownloadInfo(SqlQuery &query, SyncJournalDb::DownloadInfo * res) diff --git a/src/libsync/syncjournaldb.h b/src/libsync/syncjournaldb.h index 5aab92f55..b5521ff38 100644 --- a/src/libsync/syncjournaldb.h +++ b/src/libsync/syncjournaldb.h @@ -40,12 +40,15 @@ public: virtual ~SyncJournalDb(); SyncJournalFileRecord getFileRecord( const QString& filename ); bool setFileRecord( const SyncJournalFileRecord& record ); + + /// Like setFileRecord, but preserves checksums + bool setFileRecordMetadata( const SyncJournalFileRecord& record ); + bool deleteFileRecord( const QString& filename, bool recursively = false ); int getFileRecordCount(); bool updateFileRecordChecksum(const QString& filename, const QByteArray& transmisisonChecksum, const QByteArray& transmissionChecksumType); - bool updateFileRecordMetadata(const SyncJournalFileRecord& record); bool exists(); void walCheckpoint(); @@ -171,7 +174,6 @@ private: QScopedPointer _getFileRecordQuery; QScopedPointer _setFileRecordQuery; QScopedPointer _setFileRecordChecksumQuery; - QScopedPointer _setFileRecordMetadataQuery; QScopedPointer _getDownloadInfoQuery; QScopedPointer _setDownloadInfoQuery; QScopedPointer _deleteDownloadInfoQuery; diff --git a/test/testsyncjournaldb.h b/test/testsyncjournaldb.h index 47cd6b4be..7227c255b 100644 --- a/test/testsyncjournaldb.h +++ b/test/testsyncjournaldb.h @@ -83,7 +83,7 @@ private slots: record._remotePerm = "777"; record._mode = 12; record._fileSize = 289055; - _db.updateFileRecordMetadata(record); + _db.setFileRecordMetadata(record); storedRecord = _db.getFileRecord("foo"); QVERIFY(storedRecord == record);