VFS: Do not overwrite existing files by placeholder

For issue #7557 and #7556

Note: this change the API of the VFS plugin, so the VFS plugin needs small
adaptations
This commit is contained in:
Olivier Goffart 2019-10-30 13:16:32 +01:00 committed by Kevin Ottens
parent 9807285abd
commit 66f7b27121
No known key found for this signature in database
GPG key ID: 074BBBCB8DECC9E2
7 changed files with 114 additions and 24 deletions

View file

@ -128,6 +128,18 @@ public:
}
};
namespace detail {
struct NoResultData{};
}
template <typename Error>
class Result<void, Error> : public Result<detail::NoResultData, Error>
{
public:
using Result<detail::NoResultData, Error>::Result;
Result() : Result(detail::NoResultData{}) {}
};
namespace detail {
struct OptionalNoErrorData{};
}

View file

@ -151,20 +151,18 @@ public:
*
* If the remote metadata changes, the local placeholder's metadata should possibly
* change as well.
*
* Returning false and setting error indicates an error.
*/
virtual bool updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId, QString *error) = 0;
virtual Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) = 0;
/// Create a new dehydrated placeholder. Called from PropagateDownload.
virtual void createPlaceholder(const SyncFileItem &item) = 0;
virtual Result<void, QString> createPlaceholder(const SyncFileItem &item) = 0;
/** Convert a hydrated placeholder to a dehydrated one. Called from PropagateDownlaod.
*
* This is different from delete+create because preserving some file metadata
* (like pin states) may be essential for some vfs plugins.
*/
virtual void dehydratePlaceholder(const SyncFileItem &item) = 0;
virtual Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) = 0;
/** Discovery hook: even unchanged files may need UPDATE_METADATA.
*
@ -289,9 +287,9 @@ public:
bool socketApiPinStateActionsShown() const override { return false; }
bool isHydrating() const override { return false; }
bool updateMetadata(const QString &, time_t, qint64, const QByteArray &, QString *) override { return true; }
void createPlaceholder(const SyncFileItem &) override {}
void dehydratePlaceholder(const SyncFileItem &) override {}
Result<void, QString> updateMetadata(const QString &, time_t, qint64, const QByteArray &) override { return {}; }
Result<void, QString> createPlaceholder(const SyncFileItem &) override { return {}; }
Result<void, QString> dehydratePlaceholder(const SyncFileItem &) override { return {}; }
void convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) override {}
bool needsMetadataUpdate(const SyncFileItem &) override { return false; }

View file

@ -424,7 +424,11 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked()
}
qCDebug(lcPropagateDownload) << "dehydrating file" << _item->_file;
vfs->dehydratePlaceholder(*_item);
auto r = vfs->dehydratePlaceholder(*_item);
if (!r) {
done(SyncFileItem::NormalError, r.error());
return;
}
propagator()->_journal->deleteFileRecord(_item->_originalFile);
updateMetadata(false);
return;
@ -435,7 +439,11 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked()
}
if (_item->_type == ItemTypeVirtualFile) {
qCDebug(lcPropagateDownload) << "creating virtual file" << _item->_file;
vfs->createPlaceholder(*_item);
auto r = vfs->createPlaceholder(*_item);
if (!r) {
done(SyncFileItem::NormalError, r.error());
return;
}
updateMetadata(false);
return;
}

View file

@ -351,10 +351,10 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item)
// Update on-disk virtual file metadata
if (item->_type == ItemTypeVirtualFile) {
QString error;
if (!_syncOptions._vfs->updateMetadata(filePath, item->_modtime, item->_size, item->_fileId, &error)) {
auto r = _syncOptions._vfs->updateMetadata(filePath, item->_modtime, item->_size, item->_fileId);
if (!r) {
item->_instruction = CSYNC_INSTRUCTION_ERROR;
item->_errorString = tr("Could not update virtual file metadata: %1").arg(error);
item->_errorString = tr("Could not update virtual file metadata: %1").arg(r.error());
return;
}
}

View file

@ -68,34 +68,47 @@ bool VfsSuffix::isHydrating() const
return false;
}
bool VfsSuffix::updateMetadata(const QString &filePath, time_t modtime, qint64, const QByteArray &, QString *)
Result<void, QString> VfsSuffix::updateMetadata(const QString &filePath, time_t modtime, qint64, const QByteArray &)
{
FileSystem::setModTime(filePath, modtime);
return true;
return {};
}
void VfsSuffix::createPlaceholder(const SyncFileItem &item)
Result<void, QString> VfsSuffix::createPlaceholder(const SyncFileItem &item)
{
// The concrete shape of the placeholder is also used in isDehydratedPlaceholder() below
QString fn = _setupParams.filesystemPath + item._file;
if (!fn.endsWith(fileSuffix())) {
ASSERT(false, "vfs file isn't ending with suffix");
return;
return QString("vfs file isn't ending with suffix");
}
QFile file(fn);
file.open(QFile::ReadWrite | QFile::Truncate);
if (file.exists() && file.size() > 1
&& !FileSystem::verifyFileUnchanged(fn, item._size, item._modtime)) {
return QString("Cannot create a placeholder because a file with the placeholder name already exist");
}
if (!file.open(QFile::ReadWrite | QFile::Truncate))
return file.errorString();
file.write(" ");
file.close();
FileSystem::setModTime(fn, item._modtime);
return {};
}
void VfsSuffix::dehydratePlaceholder(const SyncFileItem &item)
Result<void, QString> VfsSuffix::dehydratePlaceholder(const SyncFileItem &item)
{
QFile::remove(_setupParams.filesystemPath + item._file);
SyncFileItem virtualItem(item);
virtualItem._file = item._renameTarget;
createPlaceholder(virtualItem);
auto r = createPlaceholder(virtualItem);
if (!r)
return r;
if (item._file != item._renameTarget) { // can be the same when renaming foo -> foo.owncloud to dehydrate
QFile::remove(_setupParams.filesystemPath + item._file);
}
// Move the item's pin state
auto pin = _setupParams.journal->internalPinStates().rawForPath(item._file.toUtf8());
@ -108,6 +121,7 @@ void VfsSuffix::dehydratePlaceholder(const SyncFileItem &item)
pin = pinState(item._renameTarget);
if (pin && *pin == PinState::AlwaysLocal)
setPinState(item._renameTarget, PinState::Unspecified);
return {};
}
void VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &)

View file

@ -38,10 +38,10 @@ public:
bool socketApiPinStateActionsShown() const override { return true; }
bool isHydrating() const override;
bool updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId, QString *error) override;
Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) override;
void createPlaceholder(const SyncFileItem &item) override;
void dehydratePlaceholder(const SyncFileItem &item) override;
Result<void, QString> createPlaceholder(const SyncFileItem &item) override;
Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) override;
void convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &) override;
bool needsMetadataUpdate(const SyncFileItem &) override { return false; }

View file

@ -603,6 +603,7 @@ private slots:
QVERIFY(!fakeFolder.currentLocalState().find("A/a1"));
QVERIFY(fakeFolder.currentLocalState().find("A/a1" DVSUFFIX));
QVERIFY(fakeFolder.currentLocalState().find("A/a1" DVSUFFIX)->size <= 1);
QVERIFY(fakeFolder.currentRemoteState().find("A/a1"));
QVERIFY(itemInstruction(completeSpy, "A/a1" DVSUFFIX, CSYNC_INSTRUCTION_SYNC));
QCOMPARE(dbRecord(fakeFolder, "A/a1" DVSUFFIX)._type, ItemTypeVirtualFile);
@ -1342,6 +1343,63 @@ private slots:
QVERIFY(fakeFolder.currentLocalState().find("online/file1"));
QVERIFY(fakeFolder.currentLocalState().find("local/file1" DVSUFFIX));
}
void testPlaceHolderExist() {
FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
fakeFolder.remoteModifier().insert("A/a1" DVSUFFIX, 111);
fakeFolder.remoteModifier().insert("A/hello" DVSUFFIX, 222);
QVERIFY(fakeFolder.syncOnce());
auto vfs = setupVfs(fakeFolder);
ItemCompletedSpy completeSpy(fakeFolder);
auto cleanup = [&]() { completeSpy.clear(); };
cleanup();
QVERIFY(fakeFolder.syncOnce());
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
QVERIFY(itemInstruction(completeSpy, "A/a1" DVSUFFIX, CSYNC_INSTRUCTION_IGNORE));
QVERIFY(itemInstruction(completeSpy, "A/hello" DVSUFFIX, CSYNC_INSTRUCTION_IGNORE));
fakeFolder.remoteModifier().insert("A/a2" DVSUFFIX);
fakeFolder.remoteModifier().insert("A/hello", 12);
fakeFolder.localModifier().insert("A/igno" DVSUFFIX, 123);
cleanup();
QVERIFY(fakeFolder.syncOnce());
QVERIFY(itemInstruction(completeSpy, "A/a1" DVSUFFIX, CSYNC_INSTRUCTION_IGNORE));
QVERIFY(itemInstruction(completeSpy, "A/igno" DVSUFFIX, CSYNC_INSTRUCTION_IGNORE));
// verify that the files are still present
QCOMPARE(fakeFolder.currentLocalState().find("A/hello" DVSUFFIX)->size, 222);
QCOMPARE(*fakeFolder.currentLocalState().find("A/hello" DVSUFFIX),
*fakeFolder.currentRemoteState().find("A/hello" DVSUFFIX));
QCOMPARE(fakeFolder.currentLocalState().find("A/igno" DVSUFFIX)->size, 123);
cleanup();
// Dehydrate
vfs->setPinState(QString(), PinState::OnlineOnly);
QVERIFY(!fakeFolder.syncOnce());
QVERIFY(itemInstruction(completeSpy, "A/igno" DVSUFFIX, CSYNC_INSTRUCTION_IGNORE));
// verify that the files are still present
QCOMPARE(fakeFolder.currentLocalState().find("A/a1" DVSUFFIX)->size, 111);
QCOMPARE(fakeFolder.currentLocalState().find("A/hello" DVSUFFIX)->size, 222);
QCOMPARE(*fakeFolder.currentLocalState().find("A/hello" DVSUFFIX),
*fakeFolder.currentRemoteState().find("A/hello" DVSUFFIX));
QCOMPARE(*fakeFolder.currentLocalState().find("A/a1"),
*fakeFolder.currentRemoteState().find("A/a1"));
QCOMPARE(fakeFolder.currentLocalState().find("A/igno" DVSUFFIX)->size, 123);
// Now disable vfs and check that all files are still there
cleanup();
SyncEngine::wipeVirtualFiles(fakeFolder.localPath(), fakeFolder.syncJournal(), *vfs);
fakeFolder.switchToVfs(QSharedPointer<Vfs>(new VfsOff));
QVERIFY(fakeFolder.syncOnce());
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
QCOMPARE(fakeFolder.currentLocalState().find("A/a1" DVSUFFIX)->size, 111);
QCOMPARE(fakeFolder.currentLocalState().find("A/hello")->size, 12);
QCOMPARE(fakeFolder.currentLocalState().find("A/hello" DVSUFFIX)->size, 222);
QCOMPARE(fakeFolder.currentLocalState().find("A/igno" DVSUFFIX)->size, 123);
}
};
QTEST_GUILESS_MAIN(TestSyncVirtualFiles)