Checksums: Treat more carefully in db #4034

In particular, preserve them on local rename or remote move.

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;

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");

src/libsync/propagatorjobs.cpp

@@ -200,13 +200,17 @@ void PropagateLocalRename::start()
         }
     }
 
+    SyncJournalFileRecord oldRecord =
+            _propagator->_journal->getFileRecord(_item->_originalFile);
     _propagator->_journal->deleteFileRecord(_item->_originalFile);
 
     // store the rename file name in the item.
     _item->_file = _item->_renameTarget;
 
     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);

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) {

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);
-
-    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;
-    }
-
-    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;
+    SyncJournalFileRecord existing = getFileRecord(record._path);
+
+    // If there's no existing record, just insert the new one.
+    if (existing._path.isEmpty()) {
+        return setFileRecord(record);
+    }
+
+    // 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)

src/libsync/syncjournaldb.h

@@ -40,12 +40,15 @@ class OWNCLOUDSYNC_EXPORT SyncJournalDb : public QObject
     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 @@ class OWNCLOUDSYNC_EXPORT SyncJournalDb : public QObject
     QScopedPointer<SqlQuery> _getFileRecordQuery;
     QScopedPointer<SqlQuery> _setFileRecordQuery;
     QScopedPointer<SqlQuery> _setFileRecordChecksumQuery;
-    QScopedPointer<SqlQuery> _setFileRecordMetadataQuery;
     QScopedPointer<SqlQuery> _getDownloadInfoQuery;
     QScopedPointer<SqlQuery> _setDownloadInfoQuery;
     QScopedPointer<SqlQuery> _deleteDownloadInfoQuery;

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);