Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit a3332ad

Browse files
TheOneRingTheOneRing
committedJan 19, 2023
Upload locked files
Fixes: #9829
1 parent 2b727b2 commit a3332ad

15 files changed

+76
-51
lines changed
 

‎changelog/unreleased/9829

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Enhancement: Sync virtual files that are locked by office etc
2+
3+
We now upload locked files again when Windows virtual files are used.
4+
This was disabled in 2.9.0 as it caused the file metadata and the locked files to get out of sync.
5+
The new solution implements explicit handling of outdated placeholders.
6+
7+
https://github.com/owncloud/client/issues/9829

‎src/common/preparedsqlquerymanager.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class OCSYNC_EXPORT PreparedSqlQuery
2929
public:
3030
~PreparedSqlQuery();
3131

32-
explicit operator bool() const { return _ok; }
32+
operator bool() const { return _ok; }
3333

3434
SqlQuery *operator->() const
3535
{
@@ -95,6 +95,8 @@ class OCSYNC_EXPORT PreparedSqlQueryManager
9595
SetPinStateQuery,
9696
WipePinStateQuery,
9797

98+
GetFileReocrdsWithDirtyPlaceholdersQuery,
99+
98100
PreparedQueryCount
99101
};
100102
PreparedSqlQueryManager() = default;

‎src/common/syncjournaldb.cpp

+45-3
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ namespace {
4646
// base query used to select file record objects, used in combination with WHERE statements.
4747
const auto getFileRecordQueryC = QByteArrayLiteral("SELECT path, inode, modtime, type, md5, fileid, remotePerm, filesize,"
4848
" ignoredChildrenRemote, contentchecksumtype.name || ':' || contentChecksum,"
49+
" hasDirtyPlaceholder"
4950
" FROM metadata"
5051
" LEFT JOIN checksumtype as contentchecksumtype ON metadata.contentChecksumTypeId == contentchecksumtype.id ");
5152
}
@@ -66,6 +67,7 @@ static void fillFileRecordFromGetQuery(SyncJournalFileRecord &rec, SqlQuery &que
6667
rec._fileSize = query.int64Value(7);
6768
rec._serverHasIgnoredFiles = (query.intValue(8) > 0);
6869
rec._checksumHeader = query.baValue(9);
70+
rec._hasDirtyPlaceholder = query.intValue(10);
6971
}
7072

7173
static QByteArray defaultJournalMode(const QString &dbPath)
@@ -384,6 +386,7 @@ bool SyncJournalDb::checkConnect()
384386
// ignoredChildrenRemote
385387
// contentChecksum
386388
// contentChecksumTypeId
389+
// hasDirtyPlaceholder
387390
"PRIMARY KEY(phash)"
388391
");");
389392

@@ -745,6 +748,22 @@ bool SyncJournalDb::updateMetadataTableStructure()
745748
commitInternal(QStringLiteral("update database structure: add contentChecksumTypeId col"));
746749
}
747750

751+
if (columns.indexOf("hasDirtyPlaceholder") == -1) {
752+
SqlQuery addDirtyQuery(_db);
753+
addDirtyQuery.prepare("ALTER TABLE metadata ADD COLUMN hasDirtyPlaceholder BOOLEAN;");
754+
if (!addDirtyQuery.exec()) {
755+
sqlFail(QStringLiteral("updateMetadataTableStructure: add hasDirtyPlaceholder column"), addDirtyQuery);
756+
re = false;
757+
}
758+
SqlQuery dirtyIndex(_db);
759+
dirtyIndex.prepare("CREATE INDEX hasDirtyPlaceholderIndex ON metadata(path) WHERE hasDirtyPlaceholder=TRUE;");
760+
if (!dirtyIndex.exec()) {
761+
sqlFail(QStringLiteral("updateMetadataTableStructure: add index on hasDirtyPlaceholder column"), dirtyIndex);
762+
re = false;
763+
}
764+
commitInternal(QStringLiteral("update database structure: add hasDirtyPlaceholder col"));
765+
}
766+
748767
auto uploadInfoColumns = tableColumns("uploadinfo");
749768
if (uploadInfoColumns.isEmpty())
750769
return false;
@@ -882,7 +901,7 @@ Result<void, QString> SyncJournalDb::setFileRecord(const SyncJournalFileRecord &
882901
qCInfo(lcDb) << "Updating file record for path:" << record._path << "inode:" << record._inode
883902
<< "modtime:" << record._modtime << "type:" << record._type
884903
<< "etag:" << record._etag << "fileId:" << record._fileId << "remotePerm:" << record._remotePerm.toString()
885-
<< "fileSize:" << record._fileSize << "checksum:" << record._checksumHeader;
904+
<< "fileSize:" << record._fileSize << "checksum:" << record._checksumHeader << "hasDirtyPlaceholder:" << record._hasDirtyPlaceholder;
886905

887906
const qint64 phash = getPHash(record._path);
888907
if (checkConnect()) {
@@ -899,8 +918,8 @@ Result<void, QString> SyncJournalDb::setFileRecord(const SyncJournalFileRecord &
899918
const auto checksumHeader = ChecksumHeader::parseChecksumHeader(record._checksumHeader);
900919
int contentChecksumTypeId = mapChecksumType(checksumHeader.type());
901920
const auto query = _queryManager.get(PreparedSqlQueryManager::SetFileRecordQuery, QByteArrayLiteral("INSERT OR REPLACE INTO metadata "
902-
"(phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize, ignoredChildrenRemote, contentChecksum, contentChecksumTypeId) "
903-
"VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7, ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16);"),
921+
"(phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize, ignoredChildrenRemote, contentChecksum, contentChecksumTypeId, hasDirtyPlaceholder) "
922+
"VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7, ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17);"),
904923
_db);
905924
if (!query) {
906925
return query->error();
@@ -922,6 +941,7 @@ Result<void, QString> SyncJournalDb::setFileRecord(const SyncJournalFileRecord &
922941
query->bindValue(14, record._serverHasIgnoredFiles ? 1 : 0);
923942
query->bindValue(15, checksumHeader.checksum());
924943
query->bindValue(16, contentChecksumTypeId);
944+
query->bindValue(17, record._hasDirtyPlaceholder);
925945

926946
if (!query->exec()) {
927947
return query->error();
@@ -2289,6 +2309,28 @@ SyncJournalDb::~SyncJournalDb()
22892309
close();
22902310
}
22912311

2312+
const QVector<SyncJournalFileRecord> SyncJournalDb::getFileRecordsWithDirtyPlaceholders() const
2313+
{
2314+
QMutexLocker locker(&_mutex);
2315+
2316+
if (OC_ENSURE(isOpen())) {
2317+
const auto query = _queryManager.get(PreparedSqlQueryManager::GetFileReocrdsWithDirtyPlaceholdersQuery, getFileRecordQueryC + QByteArrayLiteral("WHERE hasDirtyPlaceholder=TRUE"), const_cast<SyncJournalDb *>(this)->_db);
2318+
if (!OC_ENSURE(query)) {
2319+
return {};
2320+
}
2321+
if (!OC_ENSURE(query->exec())) {
2322+
return {};
2323+
}
2324+
QVector<SyncJournalFileRecord> out;
2325+
while (query->next().hasData) {
2326+
SyncJournalFileRecord rec;
2327+
fillFileRecordFromGetQuery(rec, *query);
2328+
out.append(rec);
2329+
}
2330+
return out;
2331+
}
2332+
return {};
2333+
}
22922334

22932335
bool operator==(const SyncJournalDb::DownloadInfo &lhs,
22942336
const SyncJournalDb::DownloadInfo &rhs)

‎src/common/syncjournaldb.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
6363
bool getFileRecordsByFileId(const QByteArray &fileId, const std::function<void(const SyncJournalFileRecord &)> &rowCallback);
6464
bool getFilesBelowPath(const QByteArray &path, const std::function<void(const SyncJournalFileRecord&)> &rowCallback);
6565
bool listFilesInPath(const QByteArray &path, const std::function<void(const SyncJournalFileRecord&)> &rowCallback);
66+
const QVector<SyncJournalFileRecord> getFileRecordsWithDirtyPlaceholders() const;
6667
Result<void, QString> setFileRecord(const SyncJournalFileRecord &record);
6768

6869
bool deleteFileRecord(const QString &filename, bool recursively = false);
@@ -416,7 +417,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
416417
*/
417418
QByteArray _journalMode;
418419

419-
PreparedSqlQueryManager _queryManager;
420+
mutable PreparedSqlQueryManager _queryManager;
420421

421422
/**
422423
* Whether the db was already closed, prevent recreation

‎src/common/syncjournalfilerecord.h

+1
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ class OCSYNC_EXPORT SyncJournalFileRecord
5858
qint64 _fileSize = 0;
5959
RemotePermissions _remotePerm;
6060
bool _serverHasIgnoredFiles = false;
61+
bool _hasDirtyPlaceholder = false;
6162
QByteArray _checksumHeader;
6263
};
6364

‎src/common/vfs.h

-1
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ class OCSYNC_EXPORT Vfs : public QObject
121121
};
122122
Q_ENUM(Mode)
123123
enum class ConvertToPlaceholderResult {
124-
Error,
125124
Ok,
126125
Locked
127126
};

‎src/libsync/owncloudpropagator.cpp

+6-2
Original file line numberDiff line numberDiff line change
@@ -795,10 +795,14 @@ Result<Vfs::ConvertToPlaceholderResult, QString> OwncloudPropagator::updateMetad
795795
{
796796
const QString fsPath = fullLocalPath(item.destination());
797797
const auto result = updatePlaceholder(item, fsPath, {});
798-
if (!result || result.get() != Vfs::ConvertToPlaceholderResult::Ok) {
798+
if (!result) {
799799
return result;
800800
}
801-
const auto record = item.toSyncJournalFileRecordWithInode(fsPath);
801+
auto record = item.toSyncJournalFileRecordWithInode(fsPath);
802+
if (result.get() == Vfs::ConvertToPlaceholderResult::Locked) {
803+
record._hasDirtyPlaceholder = true;
804+
Q_EMIT seenLockedFile(fullLocalPath(item._file), FileSystem::LockMode::Exclusive);
805+
}
802806
const auto dBresult = _journal->setFileRecord(record);
803807
if (!dBresult) {
804808
return dBresult.error();

‎src/libsync/propagateremotemkdir.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,6 @@ void PropagateRemoteMkdir::retryUpdateMetadata(std::chrono::steady_clock::time_p
176176
retryUpdateMetadata(start, item);
177177
});
178178
return;
179-
case Vfs::ConvertToPlaceholderResult::Error:
180-
Q_UNREACHABLE();
181179
}
182180
} else {
183181
done(SyncFileItem::FatalError, result.error());

‎src/libsync/propagateupload.cpp

-7
Original file line numberDiff line numberDiff line change
@@ -571,18 +571,11 @@ void PropagateUploadFileCommon::finalize()
571571
return;
572572
}
573573

574-
575-
#ifdef Q_OS_WIN
576-
m_fileLock.close();
577-
#endif
578574
// Update the database entry
579575
const auto result = propagator()->updateMetadata(*_item);
580576
if (!result) {
581577
done(SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error()));
582578
return;
583-
} else if (result.get() == Vfs::ConvertToPlaceholderResult::Locked) {
584-
done(SyncFileItem::SoftError, tr("The file %1 is currently in use").arg(_item->_file));
585-
return;
586579
}
587580

588581
// Files that were new on the remote shouldn't have online-only pin state

‎src/libsync/propagateupload.h

-4
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,6 @@ private slots:
255255
/** Bases headers that need to be sent on the PUT, or in the MOVE for chunking-ng */
256256
QMap<QByteArray, QByteArray> headers();
257257

258-
#ifdef Q_OS_WIN
259-
Utility::Handle m_fileLock;
260-
#endif
261-
262258
private:
263259
bool _quotaUpdated = false;
264260

‎src/libsync/propagateuploadng.cpp

+2-16
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,8 @@ void PropagateUploadFileNG::doStartUpload()
9292
const QString fileName = propagator()->fullLocalPath(_item->_file);
9393
// If the file is currently locked, we want to retry the sync
9494
// when it becomes available again.
95-
const auto lockMode = propagator()->syncOptions().requiredLockMode();
96-
if (FileSystem::isFileLocked(fileName, lockMode)) {
97-
emit propagator()->seenLockedFile(fileName, lockMode);
95+
if (FileSystem::isFileLocked(fileName, FileSystem::LockMode::Shared)) {
96+
emit propagator()->seenLockedFile(fileName, FileSystem::LockMode::Shared);
9897
abortWithError(SyncFileItem::SoftError, tr("%1 the file is currently in use").arg(QDir::toNativeSeparators(fileName)));
9998
return;
10099
}
@@ -357,19 +356,6 @@ void PropagateUploadFileNG::doFinalMove()
357356

358357
const QString source = chunkPath() + QStringLiteral("/.file");
359358

360-
#ifdef Q_OS_WIN
361-
// Try to accuire a lock on the file and keep it until we done.
362-
// If the file is locked, abort before we perform the move on the server
363-
const QString fileName = propagator()->fullLocalPath(_item->_file);
364-
const auto lockMode = propagator()->syncOptions().requiredLockMode();
365-
m_fileLock = FileSystem::lockFile(fileName, lockMode);
366-
if (!m_fileLock) {
367-
emit propagator()->seenLockedFile(fileName, lockMode);
368-
abortWithError(SyncFileItem::SoftError, tr("%1 the file is currently in use").arg(QDir::toNativeSeparators(fileName)));
369-
return;
370-
}
371-
#endif
372-
373359
auto job = new MoveJob(propagator()->account(), propagator()->account()->url(), source, destination, headers, this);
374360
addChildJob(job);
375361
connect(job, &MoveJob::finishedSignal, this, &PropagateUploadFileNG::slotMoveJobFinished);

‎src/libsync/propagateuploadtus.cpp

+2-3
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,8 @@ UploadDevice *PropagateUploadFileTUS::prepareDevice(const quint64 &chunkSize)
5959
const QString localFileName = propagator()->fullLocalPath(_item->_file);
6060
// If the file is currently locked, we want to retry the sync
6161
// when it becomes available again.
62-
const auto lockMode = propagator()->syncOptions().requiredLockMode();
63-
if (FileSystem::isFileLocked(localFileName, lockMode)) {
64-
emit propagator()->seenLockedFile(localFileName, lockMode);
62+
if (FileSystem::isFileLocked(localFileName, FileSystem::LockMode::Shared)) {
63+
emit propagator()->seenLockedFile(localFileName, FileSystem::LockMode::Shared);
6564
abortWithError(SyncFileItem::SoftError, tr("%1 the file is currently in use").arg(localFileName));
6665
return nullptr;
6766
}

‎src/libsync/propagateuploadv1.cpp

+2-3
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,8 @@ void PropagateUploadFileV1::doStartUpload()
4343
const QString fileName = propagator()->fullLocalPath(_item->_file);
4444
// If the file is currently locked, we want to retry the sync
4545
// when it becomes available again.
46-
const auto lockMode = propagator()->syncOptions().requiredLockMode();
47-
if (FileSystem::isFileLocked(fileName, lockMode)) {
48-
emit propagator()->seenLockedFile(fileName, lockMode);
46+
if (FileSystem::isFileLocked(fileName, FileSystem::LockMode::Shared)) {
47+
emit propagator()->seenLockedFile(fileName, FileSystem::LockMode::Shared);
4948
abortWithError(SyncFileItem::SoftError, tr("%1 the file is currently in use").arg(QDir::toNativeSeparators(fileName)));
5049
return;
5150
}

‎src/libsync/syncengine.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,12 @@ void SyncEngine::slotPropagationFinished(bool success)
758758

759759
conflictRecordMaintenance();
760760

761+
// update placeholders for files that where marked as dirty in a previous run
762+
qCInfo(lcEngine) << "Updating files marked as dirty";
763+
for (const auto &record : _journal->getFileRecordsWithDirtyPlaceholders()) {
764+
_propagator->updateMetadata(*SyncFileItem::fromSyncJournalFileRecord(record));
765+
}
766+
761767
_journal->deleteStaleFlagsEntries();
762768
_journal->commit(QStringLiteral("All Finished."), false);
763769

‎src/libsync/syncoptions.h

-8
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,6 @@ class OWNCLOUDSYNC_EXPORT SyncOptions
106106
void setPathPattern(const QString &pattern);
107107

108108

109-
/**
110-
* The required file lock mode
111-
*/
112-
FileSystem::LockMode requiredLockMode() const
113-
{
114-
return _vfs->mode() == Vfs::WindowsCfApi ? FileSystem::LockMode::Exclusive : FileSystem::LockMode::Shared;
115-
}
116-
117109
private:
118110
/**
119111
* Only sync files that mathc the expression

0 commit comments

Comments
 (0)
Please sign in to comment.