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 83ca5a3

Browse files
committedJan 18, 2023
Upload locked files
Fixes: #9829
1 parent 07bd9ca commit 83ca5a3

15 files changed

+70
-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+
GetFileReocrdsWithDiryPlaceholdersQuery,
99+
98100
PreparedQueryCount
99101
};
100102
PreparedSqlQueryManager() = default;

‎src/common/syncjournaldb.cpp

+39-3
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
namespace {
4646
const auto getFileRecordQueryC = QByteArrayLiteral("SELECT path, inode, modtime, type, md5, fileid, remotePerm, filesize,"
4747
" ignoredChildrenRemote, contentchecksumtype.name || ':' || contentChecksum,"
48+
" hasDirtyPlaceholder"
4849
" FROM metadata"
4950
" LEFT JOIN checksumtype as contentchecksumtype ON metadata.contentChecksumTypeId == contentchecksumtype.id ");
5051
}
@@ -65,6 +66,7 @@ static void fillFileRecordFromGetQuery(SyncJournalFileRecord &rec, SqlQuery &que
6566
rec._fileSize = query.int64Value(7);
6667
rec._serverHasIgnoredFiles = (query.intValue(8) > 0);
6768
rec._checksumHeader = query.baValue(9);
69+
rec._hasDirtyPlaceholder = query.intValue(10);
6870
}
6971

7072
static QByteArray defaultJournalMode(const QString &dbPath)
@@ -383,6 +385,7 @@ bool SyncJournalDb::checkConnect()
383385
// ignoredChildrenRemote
384386
// contentChecksum
385387
// contentChecksumTypeId
388+
// hasDirtyPlaceholder
386389
"PRIMARY KEY(phash)"
387390
");");
388391

@@ -744,6 +747,16 @@ bool SyncJournalDb::updateMetadataTableStructure()
744747
commitInternal(QStringLiteral("update database structure: add contentChecksumTypeId col"));
745748
}
746749

750+
if (columns.indexOf("hasDirtyPlaceholder") == -1) {
751+
SqlQuery query(_db);
752+
query.prepare("ALTER TABLE metadata ADD COLUMN hasDirtyPlaceholder BOOLEAN;");
753+
if (!query.exec()) {
754+
sqlFail(QStringLiteral("updateMetadataTableStructure: add hasDirtyPlaceholder column"), query);
755+
re = false;
756+
}
757+
commitInternal(QStringLiteral("update database structure: add hasDirtyPlaceholder col"));
758+
}
759+
747760
auto uploadInfoColumns = tableColumns("uploadinfo");
748761
if (uploadInfoColumns.isEmpty())
749762
return false;
@@ -881,7 +894,7 @@ Result<void, QString> SyncJournalDb::setFileRecord(const SyncJournalFileRecord &
881894
qCInfo(lcDb) << "Updating file record for path:" << record._path << "inode:" << record._inode
882895
<< "modtime:" << record._modtime << "type:" << record._type
883896
<< "etag:" << record._etag << "fileId:" << record._fileId << "remotePerm:" << record._remotePerm.toString()
884-
<< "fileSize:" << record._fileSize << "checksum:" << record._checksumHeader;
897+
<< "fileSize:" << record._fileSize << "checksum:" << record._checksumHeader << "hasDirtyPlaceholder:" << record._hasDirtyPlaceholder;
885898

886899
const qint64 phash = getPHash(record._path);
887900
if (checkConnect()) {
@@ -898,8 +911,8 @@ Result<void, QString> SyncJournalDb::setFileRecord(const SyncJournalFileRecord &
898911
const auto checksumHeader = ChecksumHeader::parseChecksumHeader(record._checksumHeader);
899912
int contentChecksumTypeId = mapChecksumType(checksumHeader.type());
900913
const auto query = _queryManager.get(PreparedSqlQueryManager::SetFileRecordQuery, QByteArrayLiteral("INSERT OR REPLACE INTO metadata "
901-
"(phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize, ignoredChildrenRemote, contentChecksum, contentChecksumTypeId) "
902-
"VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7, ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16);"),
914+
"(phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize, ignoredChildrenRemote, contentChecksum, contentChecksumTypeId, hasDirtyPlaceholder) "
915+
"VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7, ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17);"),
903916
_db);
904917
if (!query) {
905918
return query->error();
@@ -921,6 +934,7 @@ Result<void, QString> SyncJournalDb::setFileRecord(const SyncJournalFileRecord &
921934
query->bindValue(14, record._serverHasIgnoredFiles ? 1 : 0);
922935
query->bindValue(15, checksumHeader.checksum());
923936
query->bindValue(16, contentChecksumTypeId);
937+
query->bindValue(17, record._hasDirtyPlaceholder);
924938

925939
if (!query->exec()) {
926940
return query->error();
@@ -2288,6 +2302,28 @@ SyncJournalDb::~SyncJournalDb()
22882302
close();
22892303
}
22902304

2305+
const QVector<SyncJournalFileRecord> SyncJournalDb::getFileRecordsWithDirtyPlaceholders() const
2306+
{
2307+
QMutexLocker locker(&_mutex);
2308+
2309+
if (OC_ENSURE(isOpen())) {
2310+
const auto query = _queryManager.get(PreparedSqlQueryManager::GetFileReocrdsWithDiryPlaceholdersQuery, getFileRecordQueryC + QByteArrayLiteral("WHERE hasDirtyPlaceholder=TRUE"), const_cast<SyncJournalDb *>(this)->_db);
2311+
if (!OC_ENSURE(query)) {
2312+
return {};
2313+
}
2314+
if (!OC_ENSURE(query->exec())) {
2315+
return {};
2316+
}
2317+
QVector<SyncJournalFileRecord> out;
2318+
while (query->next().hasData) {
2319+
SyncJournalFileRecord rec;
2320+
fillFileRecordFromGetQuery(rec, *query);
2321+
out.append(rec);
2322+
}
2323+
return out;
2324+
}
2325+
return {};
2326+
}
22912327

22922328
bool operator==(const SyncJournalDb::DownloadInfo &lhs,
22932329
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(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.