From 798e60c9f6de853ced75e6ea6871930e36e88fd9 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Wed, 13 Dec 2017 18:04:58 +0100 Subject: On-demand downloading: Placeholder-file based prototype - Controled by an option. - New remote files start out as ItemTypePlaceholder, are created with a .owncloud extension. - When their db entry is set to ItemTypePlaceholderDownload the next sync run will download them. - Files that aren't in the placeholder state sync as usual. - See test cases in testsyncplaceholders. Missing: - User ui for triggering placeholder file download - Maybe: Going back from file to placeholder? --- src/csync/csync.h | 4 +- src/csync/csync_private.h | 5 + src/csync/csync_reconcile.cpp | 11 +- src/csync/csync_update.cpp | 75 +++++++++++-- src/libsync/account.h | 5 + src/libsync/owncloudpropagator.cpp | 5 + src/libsync/owncloudpropagator.h | 1 + src/libsync/propagatedownload.cpp | 21 ++++ src/libsync/propagatorjobs.cpp | 7 ++ src/libsync/syncengine.cpp | 4 +- test/CMakeLists.txt | 1 + test/testsyncplaceholders.cpp | 222 +++++++++++++++++++++++++++++++++++++ 12 files changed, 349 insertions(+), 12 deletions(-) create mode 100644 test/testsyncplaceholders.cpp diff --git a/src/csync/csync.h b/src/csync/csync.h index aba98ea85..5a9f49e3d 100644 --- a/src/csync/csync.h +++ b/src/csync/csync.h @@ -137,7 +137,9 @@ enum ItemType { ItemTypeFile = 0, ItemTypeSoftLink = 1, ItemTypeDirectory = 2, - ItemTypeSkip = 3 + ItemTypeSkip = 3, + ItemTypePlaceholder = 4, + ItemTypePlaceholderDownload = 5 }; diff --git a/src/csync/csync_private.h b/src/csync/csync_private.h index 2daeaa813..8089fd821 100644 --- a/src/csync/csync_private.h +++ b/src/csync/csync_private.h @@ -196,6 +196,11 @@ struct OCSYNC_EXPORT csync_s { bool upload_conflict_files = false; + /** + * Whether new remote files should start out as placeholders. + */ + bool new_files_are_placeholders = false; + csync_s(const char *localUri, OCC::SyncJournalDb *statedb); ~csync_s(); int reinitialize(); diff --git a/src/csync/csync_reconcile.cpp b/src/csync/csync_reconcile.cpp index 78c2fab03..de94e418a 100644 --- a/src/csync/csync_reconcile.cpp +++ b/src/csync/csync_reconcile.cpp @@ -141,6 +141,12 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) cur->instruction = CSYNC_INSTRUCTION_NEW; break; } + if (cur->type == ItemTypePlaceholder && ctx->current == REMOTE_REPLICA) { + /* Do not remove on the server if the local placeholder is gone: + * instead reestablish the local placeholder */ + cur->instruction = CSYNC_INSTRUCTION_NEW; + break; + } cur->instruction = CSYNC_INSTRUCTION_REMOVE; break; case CSYNC_INSTRUCTION_EVAL_RENAME: { @@ -369,7 +375,10 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) cur->instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; other->instruction = CSYNC_INSTRUCTION_NONE; } else { - cur->instruction = CSYNC_INSTRUCTION_SYNC; + if (cur->instruction != CSYNC_INSTRUCTION_NEW + && cur->instruction != CSYNC_INSTRUCTION_SYNC) { + cur->instruction = CSYNC_INSTRUCTION_SYNC; + } other->instruction = CSYNC_INSTRUCTION_NONE; } break; diff --git a/src/csync/csync_update.cpp b/src/csync/csync_update.cpp index 5a748c308..55786f9df 100644 --- a/src/csync/csync_update.cpp +++ b/src/csync/csync_update.cpp @@ -190,15 +190,29 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr f /* we have an update! */ qCInfo(lcUpdate, "Database entry found, compare: %" PRId64 " <-> %" PRId64 ", etag: %s <-> %s, inode: %" PRId64 " <-> %" PRId64 - ", size: %" PRId64 " <-> %" PRId64 ", perms: %x <-> %x, ignore: %d", + ", size: %" PRId64 " <-> %" PRId64 ", perms: %x <-> %x" + ", type: %d <-> %d, ignore: %d", ((int64_t) fs->modtime), ((int64_t) base._modtime), fs->etag.constData(), base._etag.constData(), (uint64_t) fs->inode, (uint64_t) base._inode, - (uint64_t) fs->size, (uint64_t) base._fileSize, *reinterpret_cast(&fs->remotePerm), *reinterpret_cast(&base._remotePerm), base._serverHasIgnoredFiles ); + (uint64_t) fs->size, (uint64_t) base._fileSize, + *reinterpret_cast(&fs->remotePerm), *reinterpret_cast(&base._remotePerm), + fs->type, base._type, + base._serverHasIgnoredFiles ); + if (ctx->current == REMOTE_REPLICA && base._type == ItemTypePlaceholderDownload) { + fs->instruction = CSYNC_INSTRUCTION_NEW; + fs->type = ItemTypePlaceholderDownload; + goto out; + } + if (ctx->current == REMOTE_REPLICA && fs->etag != base._etag) { fs->instruction = CSYNC_INSTRUCTION_EVAL; - // Preserve the EVAL flag later on if the type has changed. - if (base._type != fs->type) { + if (base._type == ItemTypePlaceholder && fs->type == ItemTypeFile) { + // If the local thing is a placeholder, we just update the metadata + fs->instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + fs->type = ItemTypePlaceholder; // retain the PLACEHOLDER type in the db + } else if (base._type != fs->type) { + // Preserve the EVAL flag later on if the type has changed. fs->child_modified = true; } @@ -319,10 +333,19 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr f if (!base.isValid()) return; + if (base._type == ItemTypePlaceholderDownload) { + // Remote rename of a placeholder file we have locally scheduled + // for download. We just consider this NEW but mark it for download. + fs->type = ItemTypePlaceholderDownload; + done = true; + return; + } + // Some things prohibit rename detection entirely. // Since we don't do the same checks again in reconcile, we can't // just skip the candidate, but have to give up completely. - if (base._type != fs->type) { + if (base._type != fs->type + && base._type != ItemTypePlaceholder) { qCWarning(lcUpdate, "file types different, not a rename"); done = true; return; @@ -363,6 +386,14 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr f return 1; } } + + // Potentially turn new remote files into placeholders + if (ctx->new_files_are_placeholders + && fs->instruction == CSYNC_INSTRUCTION_NEW + && fs->type == ItemTypeFile) { + fs->type = ItemTypePlaceholder; + } + goto out; } } @@ -465,7 +496,7 @@ int csync_walker(CSYNC *ctx, std::unique_ptr fs) { return rc; } -static bool fill_tree_from_db(CSYNC *ctx, const char *uri) +static bool fill_tree_from_db(CSYNC *ctx, const char *uri, bool singleFile = false) { int64_t count = 0; QByteArray skipbase; @@ -520,9 +551,19 @@ static bool fill_tree_from_db(CSYNC *ctx, const char *uri) ++count; }; - if (!ctx->statedb->getFilesBelowPath(uri, rowCallback)) { - ctx->status_code = CSYNC_STATUS_STATEDB_LOAD_ERROR; - return false; + if (singleFile) { + OCC::SyncJournalFileRecord record; + if (ctx->statedb->getFileRecord(QByteArray(uri), &record) && record.isValid()) { + rowCallback(record); + } else { + ctx->status_code = CSYNC_STATUS_STATEDB_LOAD_ERROR; + return false; + } + } else { + if (!ctx->statedb->getFilesBelowPath(uri, rowCallback)) { + ctx->status_code = CSYNC_STATUS_STATEDB_LOAD_ERROR; + return false; + } } qInfo(lcUpdate, "%" PRId64 " entries read below path %s from db.", count, uri); @@ -659,6 +700,22 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn, fullpath = QByteArray() % uri % '/' % filename; } + // When encountering placeholder files, read the relevant + // entry from the db instead. + if (ctx->current == LOCAL_REPLICA + && dirent->type == ItemTypeFile + && filename.endsWith(".owncloud")) { + QByteArray db_uri = fullpath.mid(strlen(ctx->local.uri) + 1); + db_uri = db_uri.left(db_uri.size() - 9); + if( ! fill_tree_from_db(ctx, db_uri.constData(), true) ) { + errno = ENOENT; + ctx->status_code = CSYNC_STATUS_OPENDIR_ERROR; + goto error; + } + + continue; + } + /* if the filename starts with a . we consider it a hidden file * For windows, the hidden state is also discovered within the vio * local stat function. diff --git a/src/libsync/account.h b/src/libsync/account.h index 025dca907..488ba4b45 100644 --- a/src/libsync/account.h +++ b/src/libsync/account.h @@ -233,6 +233,9 @@ public: /// Called by network jobs on credential errors, emits invalidCredentials() void handleInvalidCredentials(); + bool usePlaceholders() const { return _usePlaceholders; } + void setUsePlaceholders(bool use) { _usePlaceholders = use; } + public slots: /// Used when forgetting credentials void clearQNAMCache(); @@ -302,6 +305,8 @@ private: QString _davPath; // defaults to value from theme, might be overwritten in brandings friend class AccountManager; + + bool _usePlaceholders = false; }; } diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 026822fc8..a89d7d520 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -590,6 +590,11 @@ QString OwncloudPropagator::getFilePath(const QString &tmp_file_name) const return _localDir + tmp_file_name; } +QString OwncloudPropagator::placeholderFilePath(const QString &fileName) const +{ + return getFilePath(fileName) + QLatin1String(".owncloud"); +} + void OwncloudPropagator::scheduleNextJob() { QTimer::singleShot(0, this, &OwncloudPropagator::scheduleNextJobImpl); diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index c0d073d5a..82f61a338 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -451,6 +451,7 @@ public: bool hasCaseClashAccessibilityProblem(const QString &relfile); QString getFilePath(const QString &tmp_file_name) const; + QString placeholderFilePath(const QString &fileName) const; /** Creates the job for an item. */ diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 0601e9fba..bd313a594 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -348,6 +348,27 @@ void PropagateDownloadFile::start() qCDebug(lcPropagateDownload) << _item->_file << propagator()->_activeJobList.count(); _stopwatch.start(); + // For placeholder files just create the file and be done + if (_item->_type == ItemTypePlaceholder) { + auto fn = propagator()->placeholderFilePath(_item->_file); + qCDebug(lcPropagateDownload) << "creating placeholder file" << fn; + QFile file(fn); + file.open(QFile::ReadWrite); + file.write("stub"); + file.close(); + updateMetadata(false); + return; + } + + // If we want to download something that used to be a placeholder, + // wipe the placeholder and proceed with a normal download + if (_item->_type == ItemTypePlaceholderDownload) { + auto fn = propagator()->placeholderFilePath(_item->_file); + qCDebug(lcPropagateDownload) << "Downloading file that used to be a placeholder" << fn; + QFile::remove(fn); + _item->_type = ItemTypeFile; + } + if (_deleteExisting) { deleteExistingFolder(); diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 1634599d4..8317e9a17 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -116,6 +116,8 @@ void PropagateLocalRemove::start() return; QString filename = propagator()->_localDir + _item->_file; + if (_item->_type == ItemTypePlaceholder || _item->_type == ItemTypePlaceholderDownload) + filename = propagator()->placeholderFilePath(_item->_file); qCDebug(lcPropagateLocalRemove) << filename; @@ -224,6 +226,11 @@ void PropagateLocalRename::start() QString existingFile = propagator()->getFilePath(_item->_file); QString targetFile = propagator()->getFilePath(_item->_renameTarget); + if (_item->_type == ItemTypePlaceholder || _item->_type == ItemTypePlaceholderDownload) { + existingFile = propagator()->placeholderFilePath(_item->_file); + targetFile = propagator()->placeholderFilePath(_item->_renameTarget); + } + // if the file is a file underneath a moved dir, the _item->file is equal // to _item->renameTarget and the file is not moved as a result. if (_item->_file != _item->_renameTarget) { diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index c0423ee89..bf51eb7b8 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -612,7 +612,7 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other, if (remote) { QString filePath = _localPath + item->_file; - if (other) { + if (other && other->type != ItemTypePlaceholder && other->type != ItemTypePlaceholderDownload) { // Even if the mtime is different on the server, we always want to keep the mtime from // the file system in the DB, this is to avoid spurious upload on the next sync item->_modtime = other->modtime; @@ -849,6 +849,8 @@ void SyncEngine::startSync() return shouldDiscoverLocally(path); }; + _csync_ctx->new_files_are_placeholders = account()->usePlaceholders(); + bool ok; auto selectiveSyncBlackList = _journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &ok); if (ok) { diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 5f36690a7..96ea3e911 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -42,6 +42,7 @@ owncloud_add_test(ExcludedFiles "") owncloud_add_test(FileSystem "") owncloud_add_test(Utility "") owncloud_add_test(SyncEngine "syncenginetestutils.h") +owncloud_add_test(SyncPlaceholders "syncenginetestutils.h") owncloud_add_test(SyncMove "syncenginetestutils.h") owncloud_add_test(SyncConflict "syncenginetestutils.h") owncloud_add_test(SyncFileStatusTracker "syncenginetestutils.h") diff --git a/test/testsyncplaceholders.cpp b/test/testsyncplaceholders.cpp new file mode 100644 index 000000000..7d94ee8a5 --- /dev/null +++ b/test/testsyncplaceholders.cpp @@ -0,0 +1,222 @@ +/* + * This software is in the public domain, furnished "as is", without technical + * support, and with no warranty, express or implied, as to its usefulness for + * any purpose. + * + */ + +#include +#include "syncenginetestutils.h" +#include + +using namespace OCC; + +SyncFileItemPtr findItem(const QSignalSpy &spy, const QString &path) +{ + for (const QList &args : spy) { + auto item = args[0].value(); + if (item->destination() == path) + return item; + } + return SyncFileItemPtr(new SyncFileItem); +} + +bool itemInstruction(const QSignalSpy &spy, const QString &path, const csync_instructions_e instr) +{ + auto item = findItem(spy, path); + return item->_instruction == instr; +} + +SyncJournalFileRecord dbRecord(FakeFolder &folder, const QString &path) +{ + SyncJournalFileRecord record; + folder.syncJournal().getFileRecord(path, &record); + return record; +} + +class TestSyncPlaceholders : public QObject +{ + Q_OBJECT + +private slots: + void testPlaceholderLifecycle_data() + { + QTest::addColumn("doLocalDiscovery"); + + QTest::newRow("full local discovery") << true; + QTest::newRow("skip local discovery") << false; + } + + void testPlaceholderLifecycle() + { + QFETCH(bool, doLocalDiscovery); + + FakeFolder fakeFolder{FileInfo()}; + fakeFolder.syncEngine().account()->setUsePlaceholders(true); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); + + auto cleanup = [&]() { + completeSpy.clear(); + if (!doLocalDiscovery) + fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::DatabaseAndFilesystem); + }; + cleanup(); + + // Create a placeholder for a new remote file + fakeFolder.remoteModifier().mkdir("A"); + fakeFolder.remoteModifier().insert("A/a1", 64); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); + QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); + QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW)); + QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypePlaceholder); + cleanup(); + + // Another sync doesn't actually lead to changes + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); + QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); + QVERIFY(completeSpy.isEmpty()); + cleanup(); + + // Neither does a remote change + fakeFolder.remoteModifier().appendByte("A/a1"); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); + QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); + QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_UPDATE_METADATA)); + QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypePlaceholder); + QCOMPARE(dbRecord(fakeFolder, "A/a1")._fileSize, 65); + cleanup(); + + // If the local placeholder file is removed, it'll just be recreated + if (!doLocalDiscovery) + fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::DatabaseAndFilesystem, { "A" }); + fakeFolder.localModifier().remove("A/a1.owncloud"); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); + QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); + QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW)); + QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypePlaceholder); + QCOMPARE(dbRecord(fakeFolder, "A/a1")._fileSize, 65); + cleanup(); + + // Remote rename is propagated + fakeFolder.remoteModifier().rename("A/a1", "A/a1m"); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1m")); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a1m.owncloud")); + QVERIFY(!fakeFolder.currentRemoteState().find("A/a1")); + QVERIFY(fakeFolder.currentRemoteState().find("A/a1m")); + QVERIFY(itemInstruction(completeSpy, "A/a1m", CSYNC_INSTRUCTION_RENAME)); + QCOMPARE(dbRecord(fakeFolder, "A/a1m")._type, ItemTypePlaceholder); + cleanup(); + + // Remote remove is propagated + fakeFolder.remoteModifier().remove("A/a1m"); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1m.owncloud")); + QVERIFY(!fakeFolder.currentRemoteState().find("A/a1m")); + QVERIFY(itemInstruction(completeSpy, "A/a1m", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(!dbRecord(fakeFolder, "A/a1m").isValid()); + cleanup(); + } + + void testWithNormalSync() + { + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + fakeFolder.syncEngine().account()->setUsePlaceholders(true); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); + + auto cleanup = [&]() { + completeSpy.clear(); + }; + cleanup(); + + // No effect sync + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + cleanup(); + + // Existing files are propagated just fine in both directions + fakeFolder.localModifier().appendByte("A/a1"); + fakeFolder.localModifier().insert("A/a3"); + fakeFolder.remoteModifier().appendByte("A/a2"); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + cleanup(); + + // New files on the remote create placeholders + fakeFolder.remoteModifier().insert("A/new"); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find("A/new")); + QVERIFY(fakeFolder.currentLocalState().find("A/new.owncloud")); + QVERIFY(fakeFolder.currentRemoteState().find("A/new")); + QVERIFY(itemInstruction(completeSpy, "A/new", CSYNC_INSTRUCTION_NEW)); + QCOMPARE(dbRecord(fakeFolder, "A/new")._type, ItemTypePlaceholder); + cleanup(); + } + + void testPlaceholderDownload() + { + FakeFolder fakeFolder{FileInfo()}; + fakeFolder.syncEngine().account()->setUsePlaceholders(true); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); + + auto cleanup = [&]() { + completeSpy.clear(); + }; + cleanup(); + + auto triggerDownload = [&](const QByteArray &path) { + auto &journal = fakeFolder.syncJournal(); + SyncJournalFileRecord record; + journal.getFileRecord(path, &record); + if (!record.isValid()) + return; + record._type = ItemTypePlaceholderDownload; + journal.setFileRecord(record); + }; + + // Create a placeholder for remote files + fakeFolder.remoteModifier().mkdir("A"); + fakeFolder.remoteModifier().insert("A/a1"); + fakeFolder.remoteModifier().insert("A/a2"); + fakeFolder.remoteModifier().insert("A/a3"); + fakeFolder.remoteModifier().insert("A/a4"); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a2.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a3.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a4.owncloud")); + cleanup(); + + // Download by changing the db entry + triggerDownload("A/a1"); + triggerDownload("A/a2"); + triggerDownload("A/a3"); + triggerDownload("A/a4"); + fakeFolder.remoteModifier().appendByte("A/a2"); + fakeFolder.remoteModifier().remove("A/a3"); + fakeFolder.remoteModifier().rename("A/a4", "A/a4m"); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "A/a2", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "A/a3", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "A/a4", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "A/a4m", CSYNC_INSTRUCTION_NEW)); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + } +}; + +QTEST_GUILESS_MAIN(TestSyncPlaceholders) +#include "testsyncplaceholders.moc" -- cgit v1.2.3 From cc8497916d251b2c1aa987cb0d7f170e9a4b938f Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 15 Jan 2018 16:46:52 +0100 Subject: PlaceHolders: Trigger a download of the placeholder and open it --- src/gui/application.cpp | 27 +++++++++++++++++++++++++++ src/gui/application.h | 4 ++++ src/gui/folder.cpp | 20 ++++++++++++++++++++ src/gui/folder.h | 5 +++++ 4 files changed, 56 insertions(+) diff --git a/src/gui/application.cpp b/src/gui/application.cpp index d5acec121..033a63c64 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -458,6 +458,9 @@ void Application::parseOptions(const QStringList &options) _debugMode = true; } else if (option == QLatin1String("--version")) { _versionOnly = true; + } else if (option.endsWith(".owncloud")) { + // placeholder file, open it after the Folder were created (if the app is not terminated) + QTimer::singleShot(0, this, [this, option] { openPlaceholder(option); }); } else { showHint("Unrecognized option '" + option.toStdString() + "'"); } @@ -623,5 +626,29 @@ void Application::showSettingsDialog() _gui->slotShowSettings(); } +void Application::openPlaceholder(const QString &filename) +{ + QLatin1String placeholderExt(".owncloud"); + if (!filename.endsWith(placeholderExt)) { + qWarning(lcApplication) << "Can only handle file ending in .owncloud. Unable to open" << filename; + return; + } + QString normalName = filename.left(filename.size() - placeholderExt.size()); + auto folder = FolderMan::instance()->folderForPath(filename); + if (!folder) { + qWarning(lcApplication) << "Can't find sync folder for" << filename; + // TODO: show a QMessageBox for errors + return; + } + QString relativePath = QDir::cleanPath(normalName).mid(folder->cleanPath().length() + 1); + folder->downloadPlaceholder(relativePath); + auto con = QSharedPointer::create(); + *con = QObject::connect(folder, &Folder::syncFinished, [con, normalName] { + QObject::disconnect(*con); + if (QFile::exists(normalName)) { + QDesktopServices::openUrl(QUrl::fromLocalFile(normalName)); + } + }); +} } // namespace OCC diff --git a/src/gui/application.h b/src/gui/application.h index c61f7645e..7e5c662e5 100644 --- a/src/gui/application.h +++ b/src/gui/application.h @@ -72,6 +72,10 @@ public slots: // TODO: this should not be public void slotownCloudWizardDone(int); void slotCrash(); + /** + * Will download a placeholder file, and open the result. + */ + void openPlaceholder(const QString &filename); protected: void parseOptions(const QStringList &); diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 44666503a..4aa5141e2 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -495,6 +495,26 @@ void Folder::slotWatchedPathChanged(const QString &path) scheduleThisFolderSoon(); } +void Folder::downloadPlaceholder(const QString &_relativepath) +{ + qCInfo(lcFolder) << "Download placeholder: " << _relativepath; + auto relativepath = _relativepath.toUtf8(); + + // Set in the database that we should download the file + SyncJournalFileRecord record; + _journal.getFileRecord(relativepath, &record); + if (!record.isValid()) + return; + record._type = ItemTypePlaceholderDownload; + _journal.setFileRecord(record); + + // Make sure we go over that file during the discovery + _journal.avoidReadFromDbOnNextSync(relativepath); + + // Schedule a sync (Folder man will start the sync in a few ms) + slotScheduleThisFolder(); +} + void Folder::saveToSettings() const { // Remove first to make sure we don't get duplicates diff --git a/src/gui/folder.h b/src/gui/folder.h index d0c375261..9380bfbf5 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -282,6 +282,11 @@ public slots: */ void slotWatchedPathChanged(const QString &path); + /** + * Mark a placeholder as being ready for download, and start a sync. + */ + void downloadPlaceholder(const QString &relativepath); + private slots: void slotSyncStarted(); void slotSyncFinished(bool); -- cgit v1.2.3 From a2191474eb0883f8e912de22e7ce6bcef3c76877 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 15 Jan 2018 17:29:48 +0100 Subject: Placeholders: Move the placeholder option from the account to the folder --- src/gui/folder.cpp | 3 +++ src/gui/folder.h | 2 ++ src/libsync/account.h | 5 ----- src/libsync/syncengine.cpp | 2 +- src/libsync/syncoptions.h | 3 +++ test/testsyncplaceholders.cpp | 12 +++++++++--- 6 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 4aa5141e2..0a2979af4 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -698,6 +698,7 @@ void Folder::setSyncOptions() opt._newBigFolderSizeLimit = newFolderLimit.first ? newFolderLimit.second * 1000LL * 1000LL : -1; // convert from MB to B opt._confirmExternalStorage = cfgFile.confirmExternalStorage(); opt._moveFilesToTrash = cfgFile.moveToTrash(); + opt._usePlaceholders = _definition.usePlaceholders; QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE"); if (!chunkSizeEnv.isEmpty()) { @@ -1088,6 +1089,7 @@ void FolderDefinition::save(QSettings &settings, const FolderDefinition &folder) settings.setValue(QLatin1String("targetPath"), folder.targetPath); settings.setValue(QLatin1String("paused"), folder.paused); settings.setValue(QLatin1String("ignoreHiddenFiles"), folder.ignoreHiddenFiles); + settings.setValue(QLatin1String("usePlaceholders"), folder.usePlaceholders); // Happens only on Windows when the explorer integration is enabled. if (!folder.navigationPaneClsid.isNull()) @@ -1108,6 +1110,7 @@ bool FolderDefinition::load(QSettings &settings, const QString &alias, folder->paused = settings.value(QLatin1String("paused")).toBool(); folder->ignoreHiddenFiles = settings.value(QLatin1String("ignoreHiddenFiles"), QVariant(true)).toBool(); folder->navigationPaneClsid = settings.value(QLatin1String("navigationPaneClsid")).toUuid(); + folder->usePlaceholders = settings.value(QLatin1String("usePlaceholders")).toBool(); settings.endGroup(); // Old settings can contain paths with native separators. In the rest of the diff --git a/src/gui/folder.h b/src/gui/folder.h index 9380bfbf5..0fca5f750 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -65,6 +65,8 @@ public: bool paused; /// whether the folder syncs hidden files bool ignoreHiddenFiles; + /// New files are downloaded as placeholders + bool usePlaceholders = false; /// The CLSID where this folder appears in registry for the Explorer navigation pane entry. QUuid navigationPaneClsid; diff --git a/src/libsync/account.h b/src/libsync/account.h index 488ba4b45..025dca907 100644 --- a/src/libsync/account.h +++ b/src/libsync/account.h @@ -233,9 +233,6 @@ public: /// Called by network jobs on credential errors, emits invalidCredentials() void handleInvalidCredentials(); - bool usePlaceholders() const { return _usePlaceholders; } - void setUsePlaceholders(bool use) { _usePlaceholders = use; } - public slots: /// Used when forgetting credentials void clearQNAMCache(); @@ -305,8 +302,6 @@ private: QString _davPath; // defaults to value from theme, might be overwritten in brandings friend class AccountManager; - - bool _usePlaceholders = false; }; } diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index bf51eb7b8..3d88d2abf 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -849,7 +849,7 @@ void SyncEngine::startSync() return shouldDiscoverLocally(path); }; - _csync_ctx->new_files_are_placeholders = account()->usePlaceholders(); + _csync_ctx->new_files_are_placeholders = _syncOptions._usePlaceholders; bool ok; auto selectiveSyncBlackList = _journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &ok); diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index 676bfbeb7..e9e97efec 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -36,6 +36,9 @@ struct SyncOptions /** If remotely deleted files are needed to move to trash */ bool _moveFilesToTrash = false; + /** Create a placeholder for new files instead of downloading */ + bool _usePlaceholders = false; + /** The initial un-adjusted chunk size in bytes for chunked uploads, both * for old and new chunking algorithm, which classifies the item to be chunked * diff --git a/test/testsyncplaceholders.cpp b/test/testsyncplaceholders.cpp index 7d94ee8a5..038da9a5c 100644 --- a/test/testsyncplaceholders.cpp +++ b/test/testsyncplaceholders.cpp @@ -52,7 +52,9 @@ private slots: QFETCH(bool, doLocalDiscovery); FakeFolder fakeFolder{FileInfo()}; - fakeFolder.syncEngine().account()->setUsePlaceholders(true); + SyncOptions syncOptions; + syncOptions._usePlaceholders = true; + fakeFolder.syncEngine().setSyncOptions(syncOptions); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); @@ -132,7 +134,9 @@ private slots: void testWithNormalSync() { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; - fakeFolder.syncEngine().account()->setUsePlaceholders(true); + SyncOptions syncOptions; + syncOptions._usePlaceholders = true; + fakeFolder.syncEngine().setSyncOptions(syncOptions); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); @@ -168,7 +172,9 @@ private slots: void testPlaceholderDownload() { FakeFolder fakeFolder{FileInfo()}; - fakeFolder.syncEngine().account()->setUsePlaceholders(true); + SyncOptions syncOptions; + syncOptions._usePlaceholders = true; + fakeFolder.syncEngine().setSyncOptions(syncOptions); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); -- cgit v1.2.3 From a8b4526d650ed491e430508b96d7b5b2eb51957f Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 15 Jan 2018 17:30:33 +0100 Subject: Placeholders: Add an checkbox in the FolderWizard to enable the placeholder feature --- src/gui/accountsettings.cpp | 1 + src/gui/folderwizard.cpp | 4 ++++ src/gui/folderwizard.h | 3 +++ 3 files changed, 8 insertions(+) diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index 7f4b262f4..c4af55c8b 100644 --- a/src/gui/accountsettings.cpp +++ b/src/gui/accountsettings.cpp @@ -378,6 +378,7 @@ void AccountSettings::slotFolderWizardAccepted() folderWizard->field(QLatin1String("sourceFolder")).toString()); definition.targetPath = FolderDefinition::prepareTargetPath( folderWizard->property("targetPath").toString()); + definition.usePlaceholders = folderWizard->property("usePlaceholders").toBool(); { QDir dir(definition.localPath); diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index 549d2095f..909c7e99d 100644 --- a/src/gui/folderwizard.cpp +++ b/src/gui/folderwizard.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include @@ -481,6 +482,8 @@ FolderWizardSelectiveSync::FolderWizardSelectiveSync(const AccountPtr &account) QVBoxLayout *layout = new QVBoxLayout(this); _selectiveSync = new SelectiveSyncWidget(account, this); layout->addWidget(_selectiveSync); + _placeholderCheckBox = new QCheckBox(tr("Download placeholders instead of downloading the files (Experimental)")); + layout->addWidget(_placeholderCheckBox); } FolderWizardSelectiveSync::~FolderWizardSelectiveSync() @@ -508,6 +511,7 @@ void FolderWizardSelectiveSync::initializePage() bool FolderWizardSelectiveSync::validatePage() { wizard()->setProperty("selectiveSyncBlackList", QVariant(_selectiveSync->createBlackList())); + wizard()->setProperty("usePlaceholders", QVariant(_placeholderCheckBox->isChecked())); return true; } diff --git a/src/gui/folderwizard.h b/src/gui/folderwizard.h index b6f056f56..05602d84b 100644 --- a/src/gui/folderwizard.h +++ b/src/gui/folderwizard.h @@ -25,6 +25,8 @@ #include "ui_folderwizardsourcepage.h" #include "ui_folderwizardtargetpage.h" +class QCheckBox; + namespace OCC { class SelectiveSyncWidget; @@ -130,6 +132,7 @@ public: private: SelectiveSyncWidget *_selectiveSync; + QCheckBox *_placeholderCheckBox; }; /** -- cgit v1.2.3 From a41d3856074130e88e49387480a8d1d0006d95e2 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 15 Jan 2018 19:43:33 +0100 Subject: Placeholders: install mimetype on linux --- mirall.desktop.in | 1 + src/gui/CMakeLists.txt | 10 ++++++++++ src/gui/owncloud.xml | 7 +++++++ 3 files changed, 18 insertions(+) create mode 100644 src/gui/owncloud.xml diff --git a/mirall.desktop.in b/mirall.desktop.in index e56a9980f..89b42c4ac 100644 --- a/mirall.desktop.in +++ b/mirall.desktop.in @@ -8,6 +8,7 @@ GenericName=Folder Sync Icon=@APPLICATION_EXECUTABLE@ Keywords=@APPLICATION_NAME@;syncing;file;sharing; X-GNOME-Autostart-Delay=3 +MimeType=application/x-owncloud # Translations diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt index af08787eb..6df3abeac 100644 --- a/src/gui/CMakeLists.txt +++ b/src/gui/CMakeLists.txt @@ -351,5 +351,15 @@ if(NOT BUILD_OWNCLOUD_OSX_BUNDLE AND NOT WIN32) configure_file(${CMAKE_SOURCE_DIR}/mirall.desktop.in ${CMAKE_CURRENT_BINARY_DIR}/${APPLICATION_EXECUTABLE}.desktop) install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${APPLICATION_EXECUTABLE}.desktop DESTINATION ${DATADIR}/applications ) + + #FIXME! branding + install(FILES owncloud.xml DESTINATION ${DATADIR}/mime/packages ) + + find_package(ECM 1.2.0 CONFIG) + set(CMAKE_MODULE_PATH ${ECM_MODULE_PATH} ${ECM_KDE_MODULE_DIR} "${CMAKE_CURRENT_SOURCE_DIR}/cmake") + find_package(SharedMimeInfo) + if(SharedMimeInfo_FOUND) + update_xdg_mimetypes( ${DATADIR}/mime/packages ) + endif(SharedMimeInfo_FOUND) endif() diff --git a/src/gui/owncloud.xml b/src/gui/owncloud.xml new file mode 100644 index 000000000..7198b76bd --- /dev/null +++ b/src/gui/owncloud.xml @@ -0,0 +1,7 @@ + + + + ownCloud placeholder style + + + -- cgit v1.2.3 From e2f2c9a153a488ed83574f4e4fdd9aaefe0b0568 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Wed, 17 Jan 2018 10:48:25 +0100 Subject: Placeholders: Deal with conflicts when a placeholder exists So "foo.owncloud" exists but the user adds a new "foo". --- src/csync/csync_reconcile.cpp | 10 ++++++ src/csync/csync_update.cpp | 18 ++++++++++ test/testsyncplaceholders.cpp | 80 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/src/csync/csync_reconcile.cpp b/src/csync/csync_reconcile.cpp index de94e418a..4c29ef46b 100644 --- a/src/csync/csync_reconcile.cpp +++ b/src/csync/csync_reconcile.cpp @@ -289,6 +289,16 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) /* file on current replica is changed or new */ case CSYNC_INSTRUCTION_EVAL: case CSYNC_INSTRUCTION_NEW: + // If the db says this is a placeholder, but there is a local item, + // go to "possible conflict" mode by adjusting the remote instruction. + if (ctx->current == LOCAL_REPLICA + && (other->type == ItemTypePlaceholder || other->type == ItemTypePlaceholderDownload) + && cur->type != ItemTypePlaceholder + && other->instruction == CSYNC_INSTRUCTION_NONE) { + other->instruction = CSYNC_INSTRUCTION_EVAL; + other->type = ItemTypePlaceholderDownload; + } + switch (other->instruction) { /* file on other replica is changed or new */ case CSYNC_INSTRUCTION_NEW: diff --git a/src/csync/csync_update.cpp b/src/csync/csync_update.cpp index 55786f9df..476675621 100644 --- a/src/csync/csync_update.cpp +++ b/src/csync/csync_update.cpp @@ -198,12 +198,24 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr f *reinterpret_cast(&fs->remotePerm), *reinterpret_cast(&base._remotePerm), fs->type, base._type, base._serverHasIgnoredFiles ); + + // If the db suggests a placeholder should be downloaded, + // treat the file as new on the remote. if (ctx->current == REMOTE_REPLICA && base._type == ItemTypePlaceholderDownload) { fs->instruction = CSYNC_INSTRUCTION_NEW; fs->type = ItemTypePlaceholderDownload; goto out; } + // If what the db thinks is a placeholder is actually a file/dir, + // treat it as new locally. + if (ctx->current == LOCAL_REPLICA + && (base._type == ItemTypePlaceholder || base._type == ItemTypePlaceholderDownload) + && fs->type != ItemTypePlaceholder) { + fs->instruction = CSYNC_INSTRUCTION_EVAL; + goto out; + } + if (ctx->current == REMOTE_REPLICA && fs->etag != base._etag) { fs->instruction = CSYNC_INSTRUCTION_EVAL; @@ -707,6 +719,12 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn, && filename.endsWith(".owncloud")) { QByteArray db_uri = fullpath.mid(strlen(ctx->local.uri) + 1); db_uri = db_uri.left(db_uri.size() - 9); + + // Don't overwrite data that was already retrieved from disk! + // This can happen if foo.owncloud exists and the user adds foo. + if (ctx->local.files.findFile(db_uri)) + continue; + if( ! fill_tree_from_db(ctx, db_uri.constData(), true) ) { errno = ENOENT; ctx->status_code = CSYNC_STATUS_OPENDIR_ERROR; diff --git a/test/testsyncplaceholders.cpp b/test/testsyncplaceholders.cpp index 038da9a5c..b760354a8 100644 --- a/test/testsyncplaceholders.cpp +++ b/test/testsyncplaceholders.cpp @@ -131,6 +131,69 @@ private slots: cleanup(); } + void testPlaceholderConflict() + { + FakeFolder fakeFolder{ FileInfo() }; + SyncOptions syncOptions; + syncOptions._usePlaceholders = true; + fakeFolder.syncEngine().setSyncOptions(syncOptions); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); + + auto cleanup = [&]() { + completeSpy.clear(); + }; + cleanup(); + + Logger::instance()->setLogDebug(true); + Logger::instance()->setLogFile("-"); + + // Create a placeholder for a new remote file + fakeFolder.remoteModifier().mkdir("A"); + fakeFolder.remoteModifier().insert("A/a1", 64); + fakeFolder.remoteModifier().insert("A/a2", 64); + fakeFolder.remoteModifier().mkdir("B"); + fakeFolder.remoteModifier().insert("B/b1", 64); + fakeFolder.remoteModifier().insert("B/b2", 64); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("B/b2.owncloud")); + cleanup(); + + // A: the correct file and a conflicting file are added, placeholders stay + // B: same setup, but the placeholders are deleted by the user + fakeFolder.localModifier().insert("A/a1", 64); + fakeFolder.localModifier().insert("A/a2", 30); + fakeFolder.localModifier().insert("B/b1", 64); + fakeFolder.localModifier().insert("B/b2", 30); + fakeFolder.localModifier().remove("B/b1.owncloud"); + fakeFolder.localModifier().remove("B/b2.owncloud"); + QVERIFY(fakeFolder.syncOnce()); + + // Everything is CONFLICT since mtimes are different even for a1/b1 + QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_CONFLICT)); + QVERIFY(itemInstruction(completeSpy, "A/a2", CSYNC_INSTRUCTION_CONFLICT)); + QVERIFY(itemInstruction(completeSpy, "B/b1", CSYNC_INSTRUCTION_CONFLICT)); + QVERIFY(itemInstruction(completeSpy, "B/b2", CSYNC_INSTRUCTION_CONFLICT)); + + // no placeholder files should remain + QVERIFY(!fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(!fakeFolder.currentLocalState().find("A/a2.owncloud")); + QVERIFY(!fakeFolder.currentLocalState().find("B/b1.owncloud")); + QVERIFY(!fakeFolder.currentLocalState().find("B/b2.owncloud")); + + // conflict files should exist + QCOMPARE(fakeFolder.syncJournal().conflictRecordPaths().size(), 2); + + // nothing should have the placeholder tag + QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "A/a2")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "B/b1")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "B/b2")._type, ItemTypeFile); + + cleanup(); + } + void testWithNormalSync() { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; @@ -199,11 +262,15 @@ private slots: fakeFolder.remoteModifier().insert("A/a2"); fakeFolder.remoteModifier().insert("A/a3"); fakeFolder.remoteModifier().insert("A/a4"); + fakeFolder.remoteModifier().insert("A/a5"); + fakeFolder.remoteModifier().insert("A/a6"); QVERIFY(fakeFolder.syncOnce()); QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); QVERIFY(fakeFolder.currentLocalState().find("A/a2.owncloud")); QVERIFY(fakeFolder.currentLocalState().find("A/a3.owncloud")); QVERIFY(fakeFolder.currentLocalState().find("A/a4.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a5.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a6.owncloud")); cleanup(); // Download by changing the db entry @@ -211,16 +278,29 @@ private slots: triggerDownload("A/a2"); triggerDownload("A/a3"); triggerDownload("A/a4"); + triggerDownload("A/a5"); + triggerDownload("A/a6"); fakeFolder.remoteModifier().appendByte("A/a2"); fakeFolder.remoteModifier().remove("A/a3"); fakeFolder.remoteModifier().rename("A/a4", "A/a4m"); + fakeFolder.localModifier().insert("A/a5"); + fakeFolder.localModifier().insert("A/a6"); + fakeFolder.localModifier().remove("A/a6.owncloud"); QVERIFY(fakeFolder.syncOnce()); QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW)); QVERIFY(itemInstruction(completeSpy, "A/a2", CSYNC_INSTRUCTION_NEW)); QVERIFY(itemInstruction(completeSpy, "A/a3", CSYNC_INSTRUCTION_REMOVE)); QVERIFY(itemInstruction(completeSpy, "A/a4", CSYNC_INSTRUCTION_REMOVE)); QVERIFY(itemInstruction(completeSpy, "A/a4m", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "A/a5", CSYNC_INSTRUCTION_CONFLICT)); + QVERIFY(itemInstruction(completeSpy, "A/a6", CSYNC_INSTRUCTION_CONFLICT)); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "A/a2")._type, ItemTypeFile); + QVERIFY(!dbRecord(fakeFolder, "A/a3").isValid()); + QCOMPARE(dbRecord(fakeFolder, "A/a4m")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "A/a5")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "A/a6")._type, ItemTypeFile); } }; -- cgit v1.2.3 From 01bb241e4837d87c29d1fc8cbef82252c6e46f6f Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Thu, 25 Jan 2018 10:07:55 +0100 Subject: Placeholders: Safe migration to older client versions Now the db entries for placeholders will have the full placeholder paths. That way older clients will, on remote discovery, delete the placeholders and download the real files. --- src/csync/csync_private.h | 5 ++ src/csync/csync_reconcile.cpp | 37 +++++++---- src/csync/csync_update.cpp | 43 ++++++++++--- src/gui/folder.cpp | 2 +- src/libsync/propagatedownload.cpp | 3 +- src/libsync/propagatorjobs.cpp | 8 --- src/libsync/syncengine.cpp | 2 +- src/libsync/syncoptions.h | 2 +- test/testsyncplaceholders.cpp | 126 ++++++++++++++++++++++++++++++-------- 9 files changed, 171 insertions(+), 57 deletions(-) diff --git a/src/csync/csync_private.h b/src/csync/csync_private.h index 8089fd821..621c0a82d 100644 --- a/src/csync/csync_private.h +++ b/src/csync/csync_private.h @@ -201,6 +201,11 @@ struct OCSYNC_EXPORT csync_s { */ bool new_files_are_placeholders = false; + /** + * The suffix to use for placeholder files. + */ + QByteArray placeholder_suffix = ".owncloud"; + csync_s(const char *localUri, OCC::SyncJournalDb *statedb); ~csync_s(); int reinitialize(); diff --git a/src/csync/csync_reconcile.cpp b/src/csync/csync_reconcile.cpp index 4c29ef46b..4e43ca40e 100644 --- a/src/csync/csync_reconcile.cpp +++ b/src/csync/csync_reconcile.cpp @@ -110,7 +110,7 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) break; } - csync_file_stat_t *other = other_tree->findFile(cur->path);; + csync_file_stat_t *other = other_tree->findFile(cur->path); if (!other) { /* Check the renamed path as well. */ @@ -122,6 +122,31 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) /* If it is ignored, other->instruction will be IGNORE so this one will also be ignored */ } + // If the user adds a file locally check whether a placeholder for that name exists. + // If so, go to "potential conflict" mode by switching the remote entry to be a + // real file. + if (!other + && ctx->current == LOCAL_REPLICA + && cur->instruction == CSYNC_INSTRUCTION_NEW + && cur->type != ItemTypePlaceholder) { + // Check if we have a placeholder entry in the remote tree + auto placeholderPath = cur->path; + placeholderPath.append(ctx->placeholder_suffix); + other = other_tree->findFile(placeholderPath); + if (!other) { + /* Check the renamed path as well. */ + other = other_tree->findFile(csync_rename_adjust_parent_path(ctx, placeholderPath)); + } + if (other && other->type == ItemTypePlaceholder) { + qCInfo(lcReconcile) << "Found placeholder for local" << cur->path << "in remote tree"; + other->path = cur->path; + other->type = ItemTypePlaceholderDownload; + other->instruction = CSYNC_INSTRUCTION_EVAL; + } else { + other = nullptr; + } + } + /* file only found on current replica */ if (!other) { switch(cur->instruction) { @@ -289,16 +314,6 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) /* file on current replica is changed or new */ case CSYNC_INSTRUCTION_EVAL: case CSYNC_INSTRUCTION_NEW: - // If the db says this is a placeholder, but there is a local item, - // go to "possible conflict" mode by adjusting the remote instruction. - if (ctx->current == LOCAL_REPLICA - && (other->type == ItemTypePlaceholder || other->type == ItemTypePlaceholderDownload) - && cur->type != ItemTypePlaceholder - && other->instruction == CSYNC_INSTRUCTION_NONE) { - other->instruction = CSYNC_INSTRUCTION_EVAL; - other->type = ItemTypePlaceholderDownload; - } - switch (other->instruction) { /* file on other replica is changed or new */ case CSYNC_INSTRUCTION_NEW: diff --git a/src/csync/csync_update.cpp b/src/csync/csync_update.cpp index 476675621..d1fbc2673 100644 --- a/src/csync/csync_update.cpp +++ b/src/csync/csync_update.cpp @@ -186,6 +186,22 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr f return -1; } + // The db entry might be for a placeholder, so look for that on the + // remote side. If we find one, change the current fs to look like a + // placeholder too, because that's what one would see if the remote + // db was filled from the database. + if (ctx->current == REMOTE_REPLICA && !base.isValid() && fs->type == ItemTypeFile) { + auto placeholderPath = fs->path; + placeholderPath.append(ctx->placeholder_suffix); + ctx->statedb->getFileRecord(placeholderPath, &base); + if (base.isValid() && base._type == ItemTypePlaceholder) { + fs->type = ItemTypePlaceholder; + fs->path = placeholderPath; + } else { + base = OCC::SyncJournalFileRecord(); + } + } + if(base.isValid()) { /* there is an entry in the database */ /* we have an update! */ qCInfo(lcUpdate, "Database entry found, compare: %" PRId64 " <-> %" PRId64 @@ -219,10 +235,9 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr f if (ctx->current == REMOTE_REPLICA && fs->etag != base._etag) { fs->instruction = CSYNC_INSTRUCTION_EVAL; - if (base._type == ItemTypePlaceholder && fs->type == ItemTypeFile) { + if (fs->type == ItemTypePlaceholder) { // If the local thing is a placeholder, we just update the metadata fs->instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; - fs->type = ItemTypePlaceholder; // retain the PLACEHOLDER type in the db } else if (base._type != fs->type) { // Preserve the EVAL flag later on if the type has changed. fs->child_modified = true; @@ -369,6 +384,14 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr f return; } + // Now we know there is a sane rename candidate. + + // Rename of a placeholder + if (base._type == ItemTypePlaceholder && fs->type == ItemTypeFile) { + fs->type = ItemTypePlaceholder; + fs->path.append(ctx->placeholder_suffix); + } + // Record directory renames if (fs->type == ItemTypeDirectory) { // If the same folder was already renamed by a different entry, @@ -399,11 +422,12 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr f } } - // Potentially turn new remote files into placeholders + // Turn new remote files into placeholders if the option is enabled. if (ctx->new_files_are_placeholders && fs->instruction == CSYNC_INSTRUCTION_NEW && fs->type == ItemTypeFile) { fs->type = ItemTypePlaceholder; + fs->path.append(ctx->placeholder_suffix); } goto out; @@ -715,14 +739,15 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn, // When encountering placeholder files, read the relevant // entry from the db instead. if (ctx->current == LOCAL_REPLICA - && dirent->type == ItemTypeFile - && filename.endsWith(".owncloud")) { + && dirent->type == ItemTypeFile + && filename.endsWith(ctx->placeholder_suffix)) { QByteArray db_uri = fullpath.mid(strlen(ctx->local.uri) + 1); - db_uri = db_uri.left(db_uri.size() - 9); + QByteArray base_uri = db_uri.left(db_uri.size() - ctx->placeholder_suffix.size()); - // Don't overwrite data that was already retrieved from disk! - // This can happen if foo.owncloud exists and the user adds foo. - if (ctx->local.files.findFile(db_uri)) + // Don't fill the local tree with placeholder data if a real + // file was found. The remote tree will still have the placeholder + // file. + if (ctx->local.files.findFile(base_uri)) continue; if( ! fill_tree_from_db(ctx, db_uri.constData(), true) ) { diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 0a2979af4..b3f360baf 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -698,7 +698,7 @@ void Folder::setSyncOptions() opt._newBigFolderSizeLimit = newFolderLimit.first ? newFolderLimit.second * 1000LL * 1000LL : -1; // convert from MB to B opt._confirmExternalStorage = cfgFile.confirmExternalStorage(); opt._moveFilesToTrash = cfgFile.moveToTrash(); - opt._usePlaceholders = _definition.usePlaceholders; + opt._newFilesArePlaceholders = _definition.usePlaceholders; QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE"); if (!chunkSizeEnv.isEmpty()) { diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index bd313a594..2e5c94d97 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -350,7 +350,7 @@ void PropagateDownloadFile::start() // For placeholder files just create the file and be done if (_item->_type == ItemTypePlaceholder) { - auto fn = propagator()->placeholderFilePath(_item->_file); + auto fn = propagator()->getFilePath(_item->_file); qCDebug(lcPropagateDownload) << "creating placeholder file" << fn; QFile file(fn); file.open(QFile::ReadWrite); @@ -366,6 +366,7 @@ void PropagateDownloadFile::start() auto fn = propagator()->placeholderFilePath(_item->_file); qCDebug(lcPropagateDownload) << "Downloading file that used to be a placeholder" << fn; QFile::remove(fn); + propagator()->_journal->deleteFileRecord(_item->_file + ".owncloud"); _item->_type = ItemTypeFile; } diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 8317e9a17..b5b99f1d3 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -116,9 +116,6 @@ void PropagateLocalRemove::start() return; QString filename = propagator()->_localDir + _item->_file; - if (_item->_type == ItemTypePlaceholder || _item->_type == ItemTypePlaceholderDownload) - filename = propagator()->placeholderFilePath(_item->_file); - qCDebug(lcPropagateLocalRemove) << filename; if (propagator()->localFileNameClash(_item->_file)) { @@ -226,11 +223,6 @@ void PropagateLocalRename::start() QString existingFile = propagator()->getFilePath(_item->_file); QString targetFile = propagator()->getFilePath(_item->_renameTarget); - if (_item->_type == ItemTypePlaceholder || _item->_type == ItemTypePlaceholderDownload) { - existingFile = propagator()->placeholderFilePath(_item->_file); - targetFile = propagator()->placeholderFilePath(_item->_renameTarget); - } - // if the file is a file underneath a moved dir, the _item->file is equal // to _item->renameTarget and the file is not moved as a result. if (_item->_file != _item->_renameTarget) { diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 3d88d2abf..6ff513d39 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -849,7 +849,7 @@ void SyncEngine::startSync() return shouldDiscoverLocally(path); }; - _csync_ctx->new_files_are_placeholders = _syncOptions._usePlaceholders; + _csync_ctx->new_files_are_placeholders = _syncOptions._newFilesArePlaceholders; bool ok; auto selectiveSyncBlackList = _journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &ok); diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index e9e97efec..f442c380b 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -37,7 +37,7 @@ struct SyncOptions bool _moveFilesToTrash = false; /** Create a placeholder for new files instead of downloading */ - bool _usePlaceholders = false; + bool _newFilesArePlaceholders = false; /** The initial un-adjusted chunk size in bytes for chunked uploads, both * for old and new chunking algorithm, which classifies the item to be chunked diff --git a/test/testsyncplaceholders.cpp b/test/testsyncplaceholders.cpp index b760354a8..a62928c33 100644 --- a/test/testsyncplaceholders.cpp +++ b/test/testsyncplaceholders.cpp @@ -53,7 +53,7 @@ private slots: FakeFolder fakeFolder{FileInfo()}; SyncOptions syncOptions; - syncOptions._usePlaceholders = true; + syncOptions._newFilesArePlaceholders = true; fakeFolder.syncEngine().setSyncOptions(syncOptions); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); @@ -72,8 +72,8 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); - QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW)); - QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypePlaceholder); + QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_NEW)); + QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypePlaceholder); cleanup(); // Another sync doesn't actually lead to changes @@ -81,6 +81,17 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); + QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypePlaceholder); + QVERIFY(completeSpy.isEmpty()); + cleanup(); + + // Not even when the remote is rediscovered + fakeFolder.syncJournal().forceRemoteDiscoveryNextSync(); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); + QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); + QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypePlaceholder); QVERIFY(completeSpy.isEmpty()); cleanup(); @@ -90,9 +101,9 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); - QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_UPDATE_METADATA)); - QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypePlaceholder); - QCOMPARE(dbRecord(fakeFolder, "A/a1")._fileSize, 65); + QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_UPDATE_METADATA)); + QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypePlaceholder); + QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._fileSize, 65); cleanup(); // If the local placeholder file is removed, it'll just be recreated @@ -103,9 +114,9 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); - QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW)); - QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypePlaceholder); - QCOMPARE(dbRecord(fakeFolder, "A/a1")._fileSize, 65); + QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_NEW)); + QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypePlaceholder); + QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._fileSize, 65); cleanup(); // Remote rename is propagated @@ -117,8 +128,8 @@ private slots: QVERIFY(fakeFolder.currentLocalState().find("A/a1m.owncloud")); QVERIFY(!fakeFolder.currentRemoteState().find("A/a1")); QVERIFY(fakeFolder.currentRemoteState().find("A/a1m")); - QVERIFY(itemInstruction(completeSpy, "A/a1m", CSYNC_INSTRUCTION_RENAME)); - QCOMPARE(dbRecord(fakeFolder, "A/a1m")._type, ItemTypePlaceholder); + QVERIFY(itemInstruction(completeSpy, "A/a1m.owncloud", CSYNC_INSTRUCTION_RENAME)); + QCOMPARE(dbRecord(fakeFolder, "A/a1m.owncloud")._type, ItemTypePlaceholder); cleanup(); // Remote remove is propagated @@ -126,8 +137,9 @@ private slots: QVERIFY(fakeFolder.syncOnce()); QVERIFY(!fakeFolder.currentLocalState().find("A/a1m.owncloud")); QVERIFY(!fakeFolder.currentRemoteState().find("A/a1m")); - QVERIFY(itemInstruction(completeSpy, "A/a1m", CSYNC_INSTRUCTION_REMOVE)); - QVERIFY(!dbRecord(fakeFolder, "A/a1m").isValid()); + QVERIFY(itemInstruction(completeSpy, "A/a1m.owncloud", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/a1m.owncloud").isValid()); cleanup(); } @@ -135,7 +147,7 @@ private slots: { FakeFolder fakeFolder{ FileInfo() }; SyncOptions syncOptions; - syncOptions._usePlaceholders = true; + syncOptions._newFilesArePlaceholders = true; fakeFolder.syncEngine().setSyncOptions(syncOptions); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); @@ -145,9 +157,6 @@ private slots: }; cleanup(); - Logger::instance()->setLogDebug(true); - Logger::instance()->setLogFile("-"); - // Create a placeholder for a new remote file fakeFolder.remoteModifier().mkdir("A"); fakeFolder.remoteModifier().insert("A/a1", 64); @@ -155,6 +164,8 @@ private slots: fakeFolder.remoteModifier().mkdir("B"); fakeFolder.remoteModifier().insert("B/b1", 64); fakeFolder.remoteModifier().insert("B/b2", 64); + fakeFolder.remoteModifier().mkdir("C"); + fakeFolder.remoteModifier().insert("C/c1", 64); QVERIFY(fakeFolder.syncOnce()); QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); QVERIFY(fakeFolder.currentLocalState().find("B/b2.owncloud")); @@ -162,12 +173,15 @@ private slots: // A: the correct file and a conflicting file are added, placeholders stay // B: same setup, but the placeholders are deleted by the user + // C: user adds a *directory* locally fakeFolder.localModifier().insert("A/a1", 64); fakeFolder.localModifier().insert("A/a2", 30); fakeFolder.localModifier().insert("B/b1", 64); fakeFolder.localModifier().insert("B/b2", 30); fakeFolder.localModifier().remove("B/b1.owncloud"); fakeFolder.localModifier().remove("B/b2.owncloud"); + fakeFolder.localModifier().mkdir("C/c1"); + fakeFolder.localModifier().insert("C/c1/foo"); QVERIFY(fakeFolder.syncOnce()); // Everything is CONFLICT since mtimes are different even for a1/b1 @@ -175,21 +189,29 @@ private slots: QVERIFY(itemInstruction(completeSpy, "A/a2", CSYNC_INSTRUCTION_CONFLICT)); QVERIFY(itemInstruction(completeSpy, "B/b1", CSYNC_INSTRUCTION_CONFLICT)); QVERIFY(itemInstruction(completeSpy, "B/b2", CSYNC_INSTRUCTION_CONFLICT)); + QVERIFY(itemInstruction(completeSpy, "C/c1", CSYNC_INSTRUCTION_CONFLICT)); // no placeholder files should remain QVERIFY(!fakeFolder.currentLocalState().find("A/a1.owncloud")); QVERIFY(!fakeFolder.currentLocalState().find("A/a2.owncloud")); QVERIFY(!fakeFolder.currentLocalState().find("B/b1.owncloud")); QVERIFY(!fakeFolder.currentLocalState().find("B/b2.owncloud")); + QVERIFY(!fakeFolder.currentLocalState().find("C/c1.owncloud")); // conflict files should exist - QCOMPARE(fakeFolder.syncJournal().conflictRecordPaths().size(), 2); + QCOMPARE(fakeFolder.syncJournal().conflictRecordPaths().size(), 3); // nothing should have the placeholder tag QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypeFile); QCOMPARE(dbRecord(fakeFolder, "A/a2")._type, ItemTypeFile); QCOMPARE(dbRecord(fakeFolder, "B/b1")._type, ItemTypeFile); QCOMPARE(dbRecord(fakeFolder, "B/b2")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "C/c1")._type, ItemTypeFile); + QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/a2.owncloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "B/b1.owncloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "B/b2.owncloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "C/c1.owncloud").isValid()); cleanup(); } @@ -198,7 +220,7 @@ private slots: { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; SyncOptions syncOptions; - syncOptions._usePlaceholders = true; + syncOptions._newFilesArePlaceholders = true; fakeFolder.syncEngine().setSyncOptions(syncOptions); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); @@ -227,8 +249,8 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find("A/new")); QVERIFY(fakeFolder.currentLocalState().find("A/new.owncloud")); QVERIFY(fakeFolder.currentRemoteState().find("A/new")); - QVERIFY(itemInstruction(completeSpy, "A/new", CSYNC_INSTRUCTION_NEW)); - QCOMPARE(dbRecord(fakeFolder, "A/new")._type, ItemTypePlaceholder); + QVERIFY(itemInstruction(completeSpy, "A/new.owncloud", CSYNC_INSTRUCTION_NEW)); + QCOMPARE(dbRecord(fakeFolder, "A/new.owncloud")._type, ItemTypePlaceholder); cleanup(); } @@ -236,7 +258,7 @@ private slots: { FakeFolder fakeFolder{FileInfo()}; SyncOptions syncOptions; - syncOptions._usePlaceholders = true; + syncOptions._newFilesArePlaceholders = true; fakeFolder.syncEngine().setSyncOptions(syncOptions); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); @@ -249,7 +271,7 @@ private slots: auto triggerDownload = [&](const QByteArray &path) { auto &journal = fakeFolder.syncJournal(); SyncJournalFileRecord record; - journal.getFileRecord(path, &record); + journal.getFileRecord(path + ".owncloud", &record); if (!record.isValid()) return; record._type = ItemTypePlaceholderDownload; @@ -288,11 +310,14 @@ private slots: fakeFolder.localModifier().remove("A/a6.owncloud"); QVERIFY(fakeFolder.syncOnce()); QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_REMOVE)); QVERIFY(itemInstruction(completeSpy, "A/a2", CSYNC_INSTRUCTION_NEW)); - QVERIFY(itemInstruction(completeSpy, "A/a3", CSYNC_INSTRUCTION_REMOVE)); - QVERIFY(itemInstruction(completeSpy, "A/a4", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "A/a2.owncloud", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "A/a3.owncloud", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "A/a4.owncloud", CSYNC_INSTRUCTION_REMOVE)); QVERIFY(itemInstruction(completeSpy, "A/a4m", CSYNC_INSTRUCTION_NEW)); QVERIFY(itemInstruction(completeSpy, "A/a5", CSYNC_INSTRUCTION_CONFLICT)); + QVERIFY(itemInstruction(completeSpy, "A/a5.owncloud", CSYNC_INSTRUCTION_REMOVE)); QVERIFY(itemInstruction(completeSpy, "A/a6", CSYNC_INSTRUCTION_CONFLICT)); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypeFile); @@ -301,6 +326,57 @@ private slots: QCOMPARE(dbRecord(fakeFolder, "A/a4m")._type, ItemTypeFile); QCOMPARE(dbRecord(fakeFolder, "A/a5")._type, ItemTypeFile); QCOMPARE(dbRecord(fakeFolder, "A/a6")._type, ItemTypeFile); + QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/a2.owncloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/a3.owncloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/a4.owncloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/a5.owncloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/a6.owncloud").isValid()); + } + + // Check what might happen if an older sync client encounters placeholders + void testOldVersion() + { + FakeFolder fakeFolder{ FileInfo() }; + SyncOptions syncOptions; + syncOptions._newFilesArePlaceholders = true; + fakeFolder.syncEngine().setSyncOptions(syncOptions); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Create a placeholder + fakeFolder.remoteModifier().mkdir("A"); + fakeFolder.remoteModifier().insert("A/a1"); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); + + // Simulate an old client by switching the type of all ItemTypePlaceholder + // entries in the db to an invalid type. + auto &db = fakeFolder.syncJournal(); + SyncJournalFileRecord rec; + db.getFileRecord(QByteArray("A/a1.owncloud"), &rec); + QVERIFY(rec.isValid()); + QCOMPARE(rec._type, ItemTypePlaceholder); + rec._type = static_cast(-1); + db.setFileRecord(rec); + + // Also switch off new files becoming placeholders + syncOptions._newFilesArePlaceholders = false; + fakeFolder.syncEngine().setSyncOptions(syncOptions); + + // A sync that doesn't do remote discovery has no effect + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1")); + QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); + QVERIFY(!fakeFolder.currentRemoteState().find("A/a1.owncloud")); + + // But with a remote discovery the placeholders will be removed and + // the remote files will be downloaded. + db.forceRemoteDiscoveryNextSync(); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find("A/a1")); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1.owncloud")); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } }; -- cgit v1.2.3 From 9bd583b0e5d290ffabd05d0ab17cd7e07657a45f Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 25 Jan 2018 16:25:11 +0100 Subject: Placeholders: Fixup clicking on placeholder after previous change Now that the name in the db is the name of the placeholder file, we need to adjust the call to downloadPlaceholder --- src/gui/application.cpp | 4 ++-- src/gui/application.h | 1 + src/gui/folder.h | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 033a63c64..693aaa989 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -633,15 +633,15 @@ void Application::openPlaceholder(const QString &filename) qWarning(lcApplication) << "Can only handle file ending in .owncloud. Unable to open" << filename; return; } - QString normalName = filename.left(filename.size() - placeholderExt.size()); auto folder = FolderMan::instance()->folderForPath(filename); if (!folder) { qWarning(lcApplication) << "Can't find sync folder for" << filename; // TODO: show a QMessageBox for errors return; } - QString relativePath = QDir::cleanPath(normalName).mid(folder->cleanPath().length() + 1); + QString relativePath = QDir::cleanPath(filename).mid(folder->cleanPath().length() + 1); folder->downloadPlaceholder(relativePath); + QString normalName = filename.left(filename.size() - placeholderExt.size()); auto con = QSharedPointer::create(); *con = QObject::connect(folder, &Folder::syncFinished, [con, normalName] { QObject::disconnect(*con); diff --git a/src/gui/application.h b/src/gui/application.h index 7e5c662e5..744bc1819 100644 --- a/src/gui/application.h +++ b/src/gui/application.h @@ -74,6 +74,7 @@ public slots: void slotCrash(); /** * Will download a placeholder file, and open the result. + * The argument is the filename of the placeholder file (including the extension) */ void openPlaceholder(const QString &filename); diff --git a/src/gui/folder.h b/src/gui/folder.h index 0fca5f750..2bd8a2882 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -286,6 +286,7 @@ public slots: /** * Mark a placeholder as being ready for download, and start a sync. + * relativePath is the patch to the placeholder file (includeing the extension) */ void downloadPlaceholder(const QString &relativepath); -- cgit v1.2.3 From 87433bfe87e73f2c4f6405aa08c7cb1cb519374e Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 25 Jan 2018 16:20:35 +0100 Subject: Placeholder: The extension is now a branding option --- CMakeLists.txt | 2 ++ config.h.in | 1 + mirall.desktop.in | 2 +- src/csync/csync_private.h | 2 +- src/gui/CMakeLists.txt | 4 ++-- src/gui/application.cpp | 4 ++-- src/gui/folder.cpp | 1 + src/gui/owncloud.xml | 7 ------- src/gui/owncloud.xml.in | 7 +++++++ src/libsync/owncloudpropagator.cpp | 4 ++-- src/libsync/owncloudpropagator.h | 2 +- src/libsync/propagatedownload.cpp | 5 +++-- src/libsync/syncengine.cpp | 2 +- src/libsync/syncoptions.h | 1 + 14 files changed, 25 insertions(+), 19 deletions(-) delete mode 100644 src/gui/owncloud.xml create mode 100644 src/gui/owncloud.xml.in diff --git a/CMakeLists.txt b/CMakeLists.txt index d03426e43..7a2feb76a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -171,6 +171,8 @@ if(APPLE) set( SOCKETAPI_TEAM_IDENTIFIER_PREFIX "" CACHE STRING "SocketApi prefix (including a following dot) that must match the codesign key's TeamIdentifier/Organizational Unit" ) endif() +set(OWNCLOUD_PLACEHOLDER_SUFFIX ".owncloud" CACHE STRING "Placeholder suffix (must start with a .)") + if(BUILD_CLIENT) if(APPLE) find_package(Sparkle) diff --git a/config.h.in b/config.h.in index 804d5f68c..1f4372f2f 100644 --- a/config.h.in +++ b/config.h.in @@ -18,6 +18,7 @@ #cmakedefine APPLICATION_EXECUTABLE "@APPLICATION_EXECUTABLE@" #cmakedefine APPLICATION_UPDATE_URL "@APPLICATION_UPDATE_URL@" #cmakedefine APPLICATION_ICON_NAME "@APPLICATION_ICON_NAME@" +#cmakedefine OWNCLOUD_PLACEHOLDER_SUFFIX "@OWNCLOUD_PLACEHOLDER_SUFFIX@" #cmakedefine ZLIB_FOUND @ZLIB_FOUND@ diff --git a/mirall.desktop.in b/mirall.desktop.in index 89b42c4ac..df2506738 100644 --- a/mirall.desktop.in +++ b/mirall.desktop.in @@ -8,7 +8,7 @@ GenericName=Folder Sync Icon=@APPLICATION_EXECUTABLE@ Keywords=@APPLICATION_NAME@;syncing;file;sharing; X-GNOME-Autostart-Delay=3 -MimeType=application/x-owncloud +MimeType=application/x-@APPLICATION_EXECUTABLE@ # Translations diff --git a/src/csync/csync_private.h b/src/csync/csync_private.h index 621c0a82d..5426fba1b 100644 --- a/src/csync/csync_private.h +++ b/src/csync/csync_private.h @@ -204,7 +204,7 @@ struct OCSYNC_EXPORT csync_s { /** * The suffix to use for placeholder files. */ - QByteArray placeholder_suffix = ".owncloud"; + QByteArray placeholder_suffix; csync_s(const char *localUri, OCC::SyncJournalDb *statedb); ~csync_s(); diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt index 6df3abeac..d3edef9aa 100644 --- a/src/gui/CMakeLists.txt +++ b/src/gui/CMakeLists.txt @@ -352,8 +352,8 @@ if(NOT BUILD_OWNCLOUD_OSX_BUNDLE AND NOT WIN32) ${CMAKE_CURRENT_BINARY_DIR}/${APPLICATION_EXECUTABLE}.desktop) install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${APPLICATION_EXECUTABLE}.desktop DESTINATION ${DATADIR}/applications ) - #FIXME! branding - install(FILES owncloud.xml DESTINATION ${DATADIR}/mime/packages ) + configure_file(owncloud.xml.in ${APPLICATION_EXECUTABLE}.xml) + install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${APPLICATION_EXECUTABLE}.xml DESTINATION ${DATADIR}/mime/packages ) find_package(ECM 1.2.0 CONFIG) set(CMAKE_MODULE_PATH ${ECM_MODULE_PATH} ${ECM_KDE_MODULE_DIR} "${CMAKE_CURRENT_SOURCE_DIR}/cmake") diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 693aaa989..102a0db65 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -458,7 +458,7 @@ void Application::parseOptions(const QStringList &options) _debugMode = true; } else if (option == QLatin1String("--version")) { _versionOnly = true; - } else if (option.endsWith(".owncloud")) { + } else if (option.endsWith(QStringLiteral(OWNCLOUD_PLACEHOLDER_SUFFIX))) { // placeholder file, open it after the Folder were created (if the app is not terminated) QTimer::singleShot(0, this, [this, option] { openPlaceholder(option); }); } else { @@ -628,7 +628,7 @@ void Application::showSettingsDialog() void Application::openPlaceholder(const QString &filename) { - QLatin1String placeholderExt(".owncloud"); + QString placeholderExt = QStringLiteral(OWNCLOUD_PLACEHOLDER_SUFFIX); if (!filename.endsWith(placeholderExt)) { qWarning(lcApplication) << "Can only handle file ending in .owncloud. Unable to open" << filename; return; diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index b3f360baf..b6f2da958 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -699,6 +699,7 @@ void Folder::setSyncOptions() opt._confirmExternalStorage = cfgFile.confirmExternalStorage(); opt._moveFilesToTrash = cfgFile.moveToTrash(); opt._newFilesArePlaceholders = _definition.usePlaceholders; + opt._placeholderSuffix = QStringLiteral(OWNCLOUD_PLACEHOLDER_SUFFIX); QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE"); if (!chunkSizeEnv.isEmpty()) { diff --git a/src/gui/owncloud.xml b/src/gui/owncloud.xml deleted file mode 100644 index 7198b76bd..000000000 --- a/src/gui/owncloud.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - ownCloud placeholder style - - - diff --git a/src/gui/owncloud.xml.in b/src/gui/owncloud.xml.in new file mode 100644 index 000000000..5d1d06234 --- /dev/null +++ b/src/gui/owncloud.xml.in @@ -0,0 +1,7 @@ + + + + @APPLICATION_NAME@ placeholders + + + diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index a89d7d520..dafaec99c 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -590,9 +590,9 @@ QString OwncloudPropagator::getFilePath(const QString &tmp_file_name) const return _localDir + tmp_file_name; } -QString OwncloudPropagator::placeholderFilePath(const QString &fileName) const +QString OwncloudPropagator::addPlaceholderSuffix(const QString &fileName) const { - return getFilePath(fileName) + QLatin1String(".owncloud"); + return fileName + _syncOptions._placeholderSuffix; } void OwncloudPropagator::scheduleNextJob() diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 82f61a338..c272a06d2 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -451,7 +451,7 @@ public: bool hasCaseClashAccessibilityProblem(const QString &relfile); QString getFilePath(const QString &tmp_file_name) const; - QString placeholderFilePath(const QString &fileName) const; + QString addPlaceholderSuffix(const QString &fileName) const; /** Creates the job for an item. */ diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 2e5c94d97..77ce71386 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -363,10 +363,11 @@ void PropagateDownloadFile::start() // If we want to download something that used to be a placeholder, // wipe the placeholder and proceed with a normal download if (_item->_type == ItemTypePlaceholderDownload) { - auto fn = propagator()->placeholderFilePath(_item->_file); + auto placeholder = propagator()->addPlaceholderSuffix(_item->_file); + auto fn = propagator()->getFilePath(placeholder); qCDebug(lcPropagateDownload) << "Downloading file that used to be a placeholder" << fn; QFile::remove(fn); - propagator()->_journal->deleteFileRecord(_item->_file + ".owncloud"); + propagator()->_journal->deleteFileRecord(placeholder); _item->_type = ItemTypeFile; } diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 6ff513d39..c4197c79b 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -850,7 +850,7 @@ void SyncEngine::startSync() }; _csync_ctx->new_files_are_placeholders = _syncOptions._newFilesArePlaceholders; - + _csync_ctx->placeholder_suffix = _syncOptions._placeholderSuffix.toUtf8(); bool ok; auto selectiveSyncBlackList = _journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &ok); if (ok) { diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index f442c380b..2e302bda9 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -38,6 +38,7 @@ struct SyncOptions /** Create a placeholder for new files instead of downloading */ bool _newFilesArePlaceholders = false; + QString _placeholderSuffix; /** The initial un-adjusted chunk size in bytes for chunked uploads, both * for old and new chunking algorithm, which classifies the item to be chunked -- cgit v1.2.3 From 8b6ae04155fe0f0877ed977a4227cd28715311b5 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Fri, 26 Jan 2018 08:56:50 +0100 Subject: Placeholders: Download from shell integration --- src/gui/socketapi.cpp | 51 +++++++++++++++++++++++++++++++++++++++++++++--- src/gui/socketapi.h | 1 + src/libsync/syncengine.h | 1 + 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp index 85df1e38d..6ae3317e8 100644 --- a/src/gui/socketapi.cpp +++ b/src/gui/socketapi.cpp @@ -527,6 +527,22 @@ void SocketApi::command_OPEN_PRIVATE_LINK(const QString &localFile, SocketListen fetchPrivateLinkUrlHelper(localFile, this, &SocketApi::openPrivateLink); } +void SocketApi::command_DOWNLOAD_PLACEHOLDER(const QString &filesArg, SocketListener *) +{ + QStringList files = filesArg.split(QLatin1Char('\x1e')); // Record Separator + auto placeholderSuffix = QStringLiteral(OWNCLOUD_PLACEHOLDER_SUFFIX); + + for (const auto &file : files) { + if (!file.endsWith(placeholderSuffix)) + continue; + auto folder = FolderMan::instance()->folderForPath(file); + if (folder) { + QString relativePath = QDir::cleanPath(file).mid(folder->cleanPath().length() + 1); + folder->downloadPlaceholder(relativePath); + } + } +} + void SocketApi::copyPrivateLinkToClipboard(const QString &link) const { QApplication::clipboard()->setText(link); @@ -565,9 +581,25 @@ void SocketApi::command_GET_STRINGS(const QString &argument, SocketListener *lis void SocketApi::command_GET_MENU_ITEMS(const QString &argument, OCC::SocketListener *listener) { listener->sendMessage(QString("GET_MENU_ITEMS:BEGIN")); - bool hasSeveralFiles = argument.contains(QLatin1Char('\x1e')); // Record Separator - Folder *syncFolder = hasSeveralFiles ? nullptr : FolderMan::instance()->folderForPath(argument); - if (syncFolder && syncFolder->accountState()->isConnected()) { + QStringList files = argument.split(QLatin1Char('\x1e')); // Record Separator + + // Find the common sync folder. + // syncFolder will be null if files are in different folders. + Folder *syncFolder = nullptr; + for (const auto &file : files) { + auto folder = FolderMan::instance()->folderForPath(file); + if (folder != syncFolder) { + if (!syncFolder) { + syncFolder = folder; + } else { + syncFolder = nullptr; + break; + } + } + } + + // Sharing actions show for single files only + if (syncFolder && files.size() == 1 && syncFolder->accountState()->isConnected()) { QString systemPath = QDir::cleanPath(argument); if (systemPath.endsWith(QLatin1Char('/'))) { systemPath.truncate(systemPath.length() - 1); @@ -595,6 +627,19 @@ void SocketApi::command_GET_MENU_ITEMS(const QString &argument, OCC::SocketListe listener->sendMessage(QLatin1String("MENU_ITEM:OPEN_PRIVATE_LINK") + flagString + tr("Open in browser")); } + + // Placeholder download action + if (syncFolder) { + auto placeholderSuffix = QStringLiteral(OWNCLOUD_PLACEHOLDER_SUFFIX); + bool hasPlaceholderFile = false; + for (const auto &file : files) { + if (file.endsWith(placeholderSuffix)) + hasPlaceholderFile = true; + } + if (hasPlaceholderFile) + listener->sendMessage(QLatin1String("MENU_ITEM:DOWNLOAD_PLACEHOLDER::") + tr("Download file(s)", "", files.size())); + } + listener->sendMessage(QString("GET_MENU_ITEMS:END")); } diff --git a/src/gui/socketapi.h b/src/gui/socketapi.h index a3b131517..e3f395ff3 100644 --- a/src/gui/socketapi.h +++ b/src/gui/socketapi.h @@ -84,6 +84,7 @@ private: Q_INVOKABLE void command_COPY_PRIVATE_LINK(const QString &localFile, SocketListener *listener); Q_INVOKABLE void command_EMAIL_PRIVATE_LINK(const QString &localFile, SocketListener *listener); Q_INVOKABLE void command_OPEN_PRIVATE_LINK(const QString &localFile, SocketListener *listener); + Q_INVOKABLE void command_DOWNLOAD_PLACEHOLDER(const QString &filesArg, SocketListener *listener); /** Sends translated/branded strings that may be useful to the integration */ Q_INVOKABLE void command_GET_STRINGS(const QString &argument, SocketListener *listener); diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index 903c75598..f43e2f26b 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -76,6 +76,7 @@ public: bool isSyncRunning() const { return _syncRunning; } + SyncOptions syncOptions() const { return _syncOptions; } void setSyncOptions(const SyncOptions &options) { _syncOptions = options; } bool ignoreHiddenFiles() const { return _csync_ctx->ignore_hidden_files; } void setIgnoreHiddenFiles(bool ignore) { _csync_ctx->ignore_hidden_files = ignore; } -- cgit v1.2.3 From 8358dabbfd4b88f6fbb339186ca02936ec903456 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Fri, 26 Jan 2018 09:27:06 +0100 Subject: Folder: Simplify relative path computation It was done manually in several places after folderForPath() calls. --- src/gui/application.cpp | 4 ++-- src/gui/folderman.cpp | 8 +++++++- src/gui/folderman.h | 9 +++++++-- src/gui/owncloudgui.cpp | 4 ++-- src/gui/socketapi.cpp | 22 ++++++++++------------ 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 102a0db65..00fcfae31 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -633,13 +633,13 @@ void Application::openPlaceholder(const QString &filename) qWarning(lcApplication) << "Can only handle file ending in .owncloud. Unable to open" << filename; return; } - auto folder = FolderMan::instance()->folderForPath(filename); + QString relativePath; + auto folder = FolderMan::instance()->folderForPath(filename, &relativePath); if (!folder) { qWarning(lcApplication) << "Can't find sync folder for" << filename; // TODO: show a QMessageBox for errors return; } - QString relativePath = QDir::cleanPath(filename).mid(folder->cleanPath().length() + 1); folder->downloadPlaceholder(relativePath); QString normalName = filename.left(filename.size() - placeholderExt.size()); auto con = QSharedPointer::create(); diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index a198f79e4..ca0b08dae 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -934,7 +934,7 @@ Folder *FolderMan::addFolderInternal(FolderDefinition folderDefinition, return folder; } -Folder *FolderMan::folderForPath(const QString &path) +Folder *FolderMan::folderForPath(const QString &path, QString *relativePath) { QString absolutePath = QDir::cleanPath(path) + QLatin1Char('/'); @@ -942,10 +942,16 @@ Folder *FolderMan::folderForPath(const QString &path) const QString folderPath = folder->cleanPath() + QLatin1Char('/'); if (absolutePath.startsWith(folderPath, (Utility::isWindows() || Utility::isMac()) ? Qt::CaseInsensitive : Qt::CaseSensitive)) { + if (relativePath) { + *relativePath = absolutePath.mid(folderPath.length()); + relativePath->chop(1); // we added a '/' above + } return folder; } } + if (relativePath) + relativePath->clear(); return 0; } diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 3eb6024e2..ae9c48f89 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -77,8 +77,13 @@ public: /** Removes a folder */ void removeFolder(Folder *); - /** Returns the folder which the file or directory stored in path is in */ - Folder *folderForPath(const QString &path); + /** + * Returns the folder which the file or directory stored in path is in + * + * Optionally, the path relative to the found folder is returned in + * relativePath. + */ + Folder *folderForPath(const QString &path, QString *relativePath = nullptr); /** * returns a list of local files that exist on the local harddisk for an diff --git a/src/gui/owncloudgui.cpp b/src/gui/owncloudgui.cpp index aee68d541..16c82f19a 100644 --- a/src/gui/owncloudgui.cpp +++ b/src/gui/owncloudgui.cpp @@ -1031,7 +1031,8 @@ void ownCloudGui::raiseDialog(QWidget *raiseWidget) void ownCloudGui::slotShowShareDialog(const QString &sharePath, const QString &localPath) { - const auto folder = FolderMan::instance()->folderForPath(localPath); + QString file; + const auto folder = FolderMan::instance()->folderForPath(localPath, &file); if (!folder) { qCWarning(lcApplication) << "Could not open share dialog for" << localPath << "no responsible folder found"; return; @@ -1042,7 +1043,6 @@ void ownCloudGui::slotShowShareDialog(const QString &sharePath, const QString &l const auto accountState = folder->accountState(); - const QString file = localPath.mid(folder->cleanPath().length() + 1); SyncJournalFileRecord fileRecord; bool resharingAllowed = true; // lets assume the good diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp index 6ae3317e8..5bacc6c49 100644 --- a/src/gui/socketapi.cpp +++ b/src/gui/socketapi.cpp @@ -361,7 +361,8 @@ void SocketApi::command_RETRIEVE_FILE_STATUS(const QString &argument, SocketList { QString statusString; - Folder *syncFolder = FolderMan::instance()->folderForPath(argument); + QString relativePath; + Folder *syncFolder = FolderMan::instance()->folderForPath(argument, &relativePath); if (!syncFolder) { // this can happen in offline mode e.g.: nothing to worry about statusString = QLatin1String("NOP"); @@ -376,7 +377,6 @@ void SocketApi::command_RETRIEVE_FILE_STATUS(const QString &argument, SocketList QString directory = systemPath.left(systemPath.lastIndexOf('/')); listener->registerMonitoredDirectory(qHash(directory)); - QString relativePath = systemPath.mid(syncFolder->cleanPath().length() + 1); SyncFileStatus fileStatus = syncFolder->syncEngine().syncFileStatusTracker().fileStatus(relativePath); statusString = fileStatus.toSocketAPIString(); } @@ -389,7 +389,8 @@ void SocketApi::command_SHARE(const QString &localFile, SocketListener *listener { auto theme = Theme::instance(); - Folder *shareFolder = FolderMan::instance()->folderForPath(localFile); + QString file; + Folder *shareFolder = FolderMan::instance()->folderForPath(localFile, &file); if (!shareFolder) { const QString message = QLatin1String("SHARE:NOP:") + QDir::toNativeSeparators(localFile); // files that are not within a sync folder are not synced. @@ -403,7 +404,6 @@ void SocketApi::command_SHARE(const QString &localFile, SocketListener *listener listener->sendMessage(message); } else { const QString localFileClean = QDir::cleanPath(localFile); - const QString file = localFileClean.mid(shareFolder->cleanPath().length() + 1); SyncFileStatus fileStatus = shareFolder->syncEngine().syncFileStatusTracker().fileStatus(file); // Verify the file is on the server (to our knowledge of course) @@ -436,13 +436,13 @@ void SocketApi::command_VERSION(const QString &, SocketListener *listener) void SocketApi::command_SHARE_STATUS(const QString &localFile, SocketListener *listener) { - Folder *shareFolder = FolderMan::instance()->folderForPath(localFile); + QString file; + Folder *shareFolder = FolderMan::instance()->folderForPath(localFile, &file); if (!shareFolder) { const QString message = QLatin1String("SHARE_STATUS:NOP:") + QDir::toNativeSeparators(localFile); listener->sendMessage(message); } else { - const QString file = QDir::cleanPath(localFile).mid(shareFolder->cleanPath().length() + 1); SyncFileStatus fileStatus = shareFolder->syncEngine().syncFileStatusTracker().fileStatus(file); // Verify the file is on the server (to our knowledge of course) @@ -492,15 +492,13 @@ void SocketApi::command_SHARE_MENU_TITLE(const QString &, SocketListener *listen // Fetches the private link url asynchronously and then calls the target slot static void fetchPrivateLinkUrlHelper(const QString &localFile, SocketApi *target, void (SocketApi::*targetFun)(const QString &url) const) { - Folder *shareFolder = FolderMan::instance()->folderForPath(localFile); + QString file; + Folder *shareFolder = FolderMan::instance()->folderForPath(localFile, &file); if (!shareFolder) { qCWarning(lcSocketApi) << "Unknown path" << localFile; return; } - const QString localFileClean = QDir::cleanPath(localFile); - const QString file = localFileClean.mid(shareFolder->cleanPath().length() + 1); - AccountPtr account = shareFolder->accountState()->account(); SyncJournalFileRecord rec; @@ -535,9 +533,9 @@ void SocketApi::command_DOWNLOAD_PLACEHOLDER(const QString &filesArg, SocketList for (const auto &file : files) { if (!file.endsWith(placeholderSuffix)) continue; - auto folder = FolderMan::instance()->folderForPath(file); + QString relativePath; + auto folder = FolderMan::instance()->folderForPath(file, &relativePath); if (folder) { - QString relativePath = QDir::cleanPath(file).mid(folder->cleanPath().length() + 1); folder->downloadPlaceholder(relativePath); } } -- cgit v1.2.3 From c2f84d7c3e3963b0ce03db368a7d5913812f6ae9 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Fri, 26 Jan 2018 13:14:54 +0100 Subject: Placeholders: Ignore placeholder files in older clients To do this, we add the placeholder extension to the user exclude file automatically. However, newer clients shouldn't use that exclude pattern: so we also add version directives that allow making exclude patterns dependent on the client version. --- src/csync/csync_exclude.cpp | 62 ++++++++++++++++++++ src/csync/csync_exclude.h | 36 ++++++++++++ src/gui/application.cpp | 4 ++ test/csync/csync_tests/check_csync_exclude.cpp | 79 ++++++++++++++++++++++++++ 4 files changed, 181 insertions(+) diff --git a/src/csync/csync_exclude.cpp b/src/csync/csync_exclude.cpp index c52b1ac91..5a0933c31 100644 --- a/src/csync/csync_exclude.cpp +++ b/src/csync/csync_exclude.cpp @@ -33,9 +33,12 @@ #include "csync_misc.h" #include "common/utility.h" +#include "../version.h" #include #include +#include +#include /** Expands C-like escape sequences (in place) @@ -240,6 +243,7 @@ static CSYNC_EXCLUDE_TYPE _csync_excluded_common(const char *path, bool excludeC using namespace OCC; ExcludedFiles::ExcludedFiles() + : _clientVersion(MIRALL_VERSION_MAJOR, MIRALL_VERSION_MINOR, MIRALL_VERSION_PATCH) { // Windows used to use PathMatchSpec which allows *foo to match abc/deffoo. _wildcardsMatchSlash = Utility::isWindows(); @@ -278,6 +282,34 @@ void ExcludedFiles::setWildcardsMatchSlash(bool onoff) prepare(); } +void ExcludedFiles::setClientVersion(ExcludedFiles::Version version) +{ + _clientVersion = version; +} + +void ExcludedFiles::setupPlaceholderExclude( + const QString &excludeFile, const QByteArray &placeholderExtension) +{ + if (!QFile::exists(excludeFile)) { + // Ensure the parent paths exist + QDir().mkpath(QFileInfo(excludeFile).dir().absolutePath()); + } else { + // Does the exclude file contain the exclude already? + QFile file(excludeFile); + file.open(QIODevice::ReadOnly | QIODevice::Text); + auto data = file.readAll(); + file.close(); + if (data.contains("\n*" + placeholderExtension + "\n")) + return; + } + + // Add it to the file + QFile file(excludeFile); + file.open(QIODevice::ReadWrite | QIODevice::Append); + file.write("\n#!version < 2.5.0\n*" + placeholderExtension + "\n"); + file.close(); +} + bool ExcludedFiles::reloadExcludeFiles() { _allExcludes.clear(); @@ -290,6 +322,10 @@ bool ExcludedFiles::reloadExcludeFiles() } while (!f.atEnd()) { QByteArray line = f.readLine().trimmed(); + if (line.startsWith("#!version")) { + if (!versionDirectiveKeepNextLine(line)) + f.readLine(); + } if (line.isEmpty() || line.startsWith('#')) continue; csync_exclude_expand_escapes(line); @@ -301,6 +337,32 @@ bool ExcludedFiles::reloadExcludeFiles() return success; } +bool ExcludedFiles::versionDirectiveKeepNextLine(const QByteArray &directive) const +{ + if (!directive.startsWith("#!version")) + return true; + QByteArrayList args = directive.split(' '); + if (args.size() != 3) + return true; + QByteArray op = args[1]; + QByteArrayList argVersions = args[2].split('.'); + if (argVersions.size() != 3) + return true; + + auto argVersion = std::make_tuple(argVersions[0].toInt(), argVersions[1].toInt(), argVersions[2].toInt()); + if (op == "<=") + return _clientVersion <= argVersion; + if (op == "<") + return _clientVersion < argVersion; + if (op == ">") + return _clientVersion > argVersion; + if (op == ">=") + return _clientVersion >= argVersion; + if (op == "==") + return _clientVersion == argVersion; + return true; +} + bool ExcludedFiles::isExcluded( const QString &filePath, const QString &basePath, diff --git a/src/csync/csync_exclude.h b/src/csync/csync_exclude.h index 249ec7bff..fba766b6f 100644 --- a/src/csync/csync_exclude.h +++ b/src/csync/csync_exclude.h @@ -66,6 +66,8 @@ class OCSYNC_EXPORT ExcludedFiles : public QObject { Q_OBJECT public: + typedef std::tuple Version; + ExcludedFiles(); ~ExcludedFiles(); @@ -114,6 +116,11 @@ public: */ void setWildcardsMatchSlash(bool onoff); + /** + * Sets the client version, only used for testing. + */ + void setClientVersion(Version version); + /** * Generate a hook for traversal exclude pattern matching * that csync can use. @@ -124,6 +131,12 @@ public: auto csyncTraversalMatchFun() const -> std::function; + /** + * Adds the exclude that skips placeholder files in older versions + * to the user exclude file. + */ + static void setupPlaceholderExclude(const QString &excludeFile, const QByteArray &placeholderExtension); + public slots: /** * Reloads the exclude patterns from the registered paths. @@ -131,6 +144,23 @@ public slots: bool reloadExcludeFiles(); private: + /** + * Returns true if the version directive indicates the next line + * should be skipped. + * + * A version directive has the form "#!version " + * where can be <, <=, ==, >, >= and can be any version + * like 2.5.0. + * + * Example: + * + * #!version < 2.5.0 + * myexclude + * + * Would enable the "myexclude" pattern only for versions before 2.5.0. + */ + bool versionDirectiveKeepNextLine(const QByteArray &directive) const; + /** * @brief Match the exclude pattern against the full path. * @@ -216,6 +246,12 @@ private: */ bool _wildcardsMatchSlash = false; + /** + * The client version. Used to evaluate version-dependent excludes, + * see versionDirectiveKeepNextLine(). + */ + Version _clientVersion; + friend class ExcludedFilesTest; }; diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 00fcfae31..24096c2a1 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -37,6 +37,7 @@ #include "updater/ocupdater.h" #include "owncloudsetupwizard.h" #include "version.h" +#include "csync_exclude.h" #include "config.h" @@ -172,6 +173,9 @@ Application::Application(int &argc, char **argv) if (!AbstractNetworkJob::httpTimeout) AbstractNetworkJob::httpTimeout = cfg.timeout(); + ExcludedFiles::setupPlaceholderExclude( + cfg.excludeFile(ConfigFile::UserScope), OWNCLOUD_PLACEHOLDER_SUFFIX); + _folderManager.reset(new FolderMan); connect(this, &SharedTools::QtSingleApplication::messageReceived, this, &Application::slotParseMessage); diff --git a/test/csync/csync_tests/check_csync_exclude.cpp b/test/csync/csync_tests/check_csync_exclude.cpp index f719aa4aa..3fb658f2d 100644 --- a/test/csync/csync_tests/check_csync_exclude.cpp +++ b/test/csync/csync_tests/check_csync_exclude.cpp @@ -22,6 +22,8 @@ #include #include +#include + #define CSYNC_TEST 1 #include "csync_exclude.cpp" @@ -624,6 +626,81 @@ static void check_csync_exclude_expand_escapes(void **state) assert_true(0 == strcmp(line.constData(), "\\")); } +static void check_placeholder_exclude(void **state) +{ + (void)state; + + auto readFile = [](const QString &file) { + QFile f(file); + f.open(QIODevice::ReadOnly | QIODevice::Text); + return f.readAll(); + }; + + QTemporaryDir tempDir; + QString path; + QByteArray expected = "\n#!version < 2.5.0\n*.owncloud\n"; + + // Case 1: No file exists yet, parent dirs are missing too + path = tempDir.filePath("foo/bar/exclude.lst"); + ExcludedFiles::setupPlaceholderExclude(path, ".owncloud"); + + assert_true(QFile::exists(path)); + assert_true(readFile(path) == expected); + + // Case 2: Running it again + ExcludedFiles::setupPlaceholderExclude(path, ".owncloud"); + assert_true(readFile(path) == expected); + + // Case 3: File exists, has some data + { + QFile f(path); + f.open(QIODevice::WriteOnly | QIODevice::Truncate); + f.write("# bla\nmyexclude\n\nanotherexclude"); + f.close(); + } + ExcludedFiles::setupPlaceholderExclude(path, ".owncloud"); + assert_true(readFile(path) == "# bla\nmyexclude\n\nanotherexclude" + expected); + + // Case 4: Running it again still does nothing + ExcludedFiles::setupPlaceholderExclude(path, ".owncloud"); + assert_true(readFile(path) == "# bla\nmyexclude\n\nanotherexclude" + expected); + + // Case 5: Verify that reading this file doesn't actually include the exclude + ExcludedFiles excludes; + excludes.addExcludeFilePath(path); + excludes.reloadExcludeFiles(); + assert_false(excludes._allExcludes.contains("*.owncloud")); + assert_true(excludes._allExcludes.contains("myexclude")); +} + +static void check_version_directive(void **state) +{ + (void)state; + + ExcludedFiles excludes; + excludes.setClientVersion(ExcludedFiles::Version(2, 5, 0)); + + std::vector> tests = { + { "#!version == 2.5.0", true }, + { "#!version == 2.6.0", false }, + { "#!version < 2.6.0", true }, + { "#!version <= 2.6.0", true }, + { "#!version > 2.6.0", false }, + { "#!version >= 2.6.0", false }, + { "#!version < 2.4.0", false }, + { "#!version <= 2.4.0", false }, + { "#!version > 2.4.0", true }, + { "#!version >= 2.4.0", true }, + { "#!version < 2.5.0", false }, + { "#!version <= 2.5.0", true }, + { "#!version > 2.5.0", false }, + { "#!version >= 2.5.0", true }, + }; + for (auto test : tests) { + assert_true(excludes.versionDirectiveKeepNextLine(test.first) == test.second); + } +} + }; // class ExcludedFilesTest int torture_run_tests(void) @@ -642,6 +719,8 @@ int torture_run_tests(void) cmocka_unit_test_setup_teardown(T::check_csync_is_windows_reserved_word, T::setup_init, T::teardown), cmocka_unit_test_setup_teardown(T::check_csync_excluded_performance, T::setup_init, T::teardown), cmocka_unit_test(T::check_csync_exclude_expand_escapes), + cmocka_unit_test(T::check_placeholder_exclude), + cmocka_unit_test(T::check_version_directive), }; return cmocka_run_group_tests(tests, NULL, NULL); -- cgit v1.2.3 From fb3e3edcbdc4ddf64f7f13eddc88a279f044df23 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Mon, 29 Jan 2018 11:16:50 +0100 Subject: SyncOptions: Add default placeholder suffix Otherwise each test has to set this up anew. --- src/libsync/syncengine.cpp | 6 ++++++ src/libsync/syncoptions.h | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index c4197c79b..0f9907ca4 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -851,6 +851,12 @@ void SyncEngine::startSync() _csync_ctx->new_files_are_placeholders = _syncOptions._newFilesArePlaceholders; _csync_ctx->placeholder_suffix = _syncOptions._placeholderSuffix.toUtf8(); + if (_csync_ctx->new_files_are_placeholders && _csync_ctx->placeholder_suffix.isEmpty()) { + csyncError(tr("Using placeholder files, but placeholder suffix is not set")); + finalize(false); + return; + } + bool ok; auto selectiveSyncBlackList = _journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &ok); if (ok) { diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index 2e302bda9..ea48cd83e 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -38,7 +38,7 @@ struct SyncOptions /** Create a placeholder for new files instead of downloading */ bool _newFilesArePlaceholders = false; - QString _placeholderSuffix; + QString _placeholderSuffix = ".owncloud"; /** The initial un-adjusted chunk size in bytes for chunked uploads, both * for old and new chunking algorithm, which classifies the item to be chunked -- cgit v1.2.3 From 65a2b1aa8a5b43ae8c867c0c9afd24e17f577c8e Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Mon, 29 Jan 2018 13:02:31 +0100 Subject: Placeholders: Fix migration issues Some edgecases weren't covered and didn't have tests yet. --- src/csync/csync_reconcile.cpp | 20 +++++++++++++--- src/csync/csync_update.cpp | 13 +++------- test/testsyncplaceholders.cpp | 55 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/src/csync/csync_reconcile.cpp b/src/csync/csync_reconcile.cpp index 4e43ca40e..cf254ff57 100644 --- a/src/csync/csync_reconcile.cpp +++ b/src/csync/csync_reconcile.cpp @@ -166,9 +166,12 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) cur->instruction = CSYNC_INSTRUCTION_NEW; break; } - if (cur->type == ItemTypePlaceholder && ctx->current == REMOTE_REPLICA) { - /* Do not remove on the server if the local placeholder is gone: - * instead reestablish the local placeholder */ + /* If the local placeholder is gone it should be reestablished. + * Unless the base file is seen in the local tree now. */ + if (cur->type == ItemTypePlaceholder + && ctx->current == REMOTE_REPLICA + && cur->path.endsWith(ctx->placeholder_suffix) + && !other_tree->findFile(cur->path.left(cur->path.size() - ctx->placeholder_suffix.size()))) { cur->instruction = CSYNC_INSTRUCTION_NEW; break; } @@ -422,6 +425,17 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) if (cur->instruction == CSYNC_INSTRUCTION_EVAL) cur->instruction = CSYNC_INSTRUCTION_NEW; break; + case CSYNC_INSTRUCTION_NONE: + // NONE/NONE on placeholders might become a REMOVE if the base file + // is found in the local tree. + if (cur->type == ItemTypePlaceholder + && other->instruction == CSYNC_INSTRUCTION_NONE + && ctx->current == LOCAL_REPLICA + && cur->path.endsWith(ctx->placeholder_suffix) + && ctx->local.files.findFile(cur->path.left(cur->path.size() - ctx->placeholder_suffix.size()))) { + cur->instruction = CSYNC_INSTRUCTION_REMOVE; + } + break; default: break; } diff --git a/src/csync/csync_update.cpp b/src/csync/csync_update.cpp index d1fbc2673..4de7dcb90 100644 --- a/src/csync/csync_update.cpp +++ b/src/csync/csync_update.cpp @@ -47,6 +47,7 @@ #include "common/asserts.h" #include +#include // Needed for PRIu64 on MinGW in C++ mode. #define __STDC_FORMAT_MACROS @@ -742,18 +743,10 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn, && dirent->type == ItemTypeFile && filename.endsWith(ctx->placeholder_suffix)) { QByteArray db_uri = fullpath.mid(strlen(ctx->local.uri) + 1); - QByteArray base_uri = db_uri.left(db_uri.size() - ctx->placeholder_suffix.size()); - - // Don't fill the local tree with placeholder data if a real - // file was found. The remote tree will still have the placeholder - // file. - if (ctx->local.files.findFile(base_uri)) - continue; if( ! fill_tree_from_db(ctx, db_uri.constData(), true) ) { - errno = ENOENT; - ctx->status_code = CSYNC_STATUS_OPENDIR_ERROR; - goto error; + qCWarning(lcUpdate) << "Placeholder without db entry for" << filename; + QFile::remove(fullpath); } continue; diff --git a/test/testsyncplaceholders.cpp b/test/testsyncplaceholders.cpp index a62928c33..454ef45ac 100644 --- a/test/testsyncplaceholders.cpp +++ b/test/testsyncplaceholders.cpp @@ -141,6 +141,26 @@ private slots: QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid()); QVERIFY(!dbRecord(fakeFolder, "A/a1m.owncloud").isValid()); cleanup(); + + // Edge case: Local placeholder but no db entry for some reason + fakeFolder.remoteModifier().insert("A/a2", 64); + fakeFolder.remoteModifier().insert("A/a3", 64); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find("A/a2.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a3.owncloud")); + cleanup(); + + fakeFolder.syncEngine().journal()->deleteFileRecord("A/a2.owncloud"); + fakeFolder.syncEngine().journal()->deleteFileRecord("A/a3.owncloud"); + fakeFolder.remoteModifier().remove("A/a3"); + fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::FilesystemOnly); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find("A/a2.owncloud")); + QVERIFY(itemInstruction(completeSpy, "A/a2.owncloud", CSYNC_INSTRUCTION_NEW)); + QVERIFY(dbRecord(fakeFolder, "A/a2.owncloud").isValid()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a3.owncloud")); + QVERIFY(!dbRecord(fakeFolder, "A/a3.owncloud").isValid()); + cleanup(); } void testPlaceholderConflict() @@ -335,7 +355,7 @@ private slots: } // Check what might happen if an older sync client encounters placeholders - void testOldVersion() + void testOldVersion1() { FakeFolder fakeFolder{ FileInfo() }; SyncOptions syncOptions; @@ -378,6 +398,39 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find("A/a1.owncloud")); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + + // Older versions may leave db entries for foo and foo.owncloud + void testOldVersion2() + { + FakeFolder fakeFolder{ FileInfo() }; + + // Sync a file + fakeFolder.remoteModifier().mkdir("A"); + fakeFolder.remoteModifier().insert("A/a1"); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Create the placeholder too + // In the wild, the new version would create the placeholder and the db entry + // while the old version would download the plain file. + fakeFolder.localModifier().insert("A/a1.owncloud"); + auto &db = fakeFolder.syncJournal(); + SyncJournalFileRecord rec; + db.getFileRecord(QByteArray("A/a1"), &rec); + rec._type = ItemTypePlaceholder; + rec._path = "A/a1.owncloud"; + db.setFileRecord(rec); + + SyncOptions syncOptions; + syncOptions._newFilesArePlaceholders = true; + fakeFolder.syncEngine().setSyncOptions(syncOptions); + + // Check that a sync removes the placeholder and its db entry + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1.owncloud")); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid()); + } }; QTEST_GUILESS_MAIN(TestSyncPlaceholders) -- cgit v1.2.3 From 70b4dc99dc735511c2402ae066f3c7da7af06408 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Wed, 21 Mar 2018 12:23:31 +0100 Subject: NSIS: Register placeholder extension Also change the placeholder suffix config option to not include the dot, the dotless form is needed in the nsis script. --- CMakeLists.txt | 2 - OWNCLOUD.cmake | 1 + admin/win/nsi/lib/fileassoc.nsh | 120 ++++++++++++++++++++++++++++++++++++++++ cmake/modules/NSIS.template.in | 10 ++++ config.h.in | 3 +- src/gui/application.cpp | 6 +- src/gui/folder.cpp | 2 +- src/gui/owncloud.xml.in | 2 +- src/gui/socketapi.cpp | 4 +- 9 files changed, 140 insertions(+), 10 deletions(-) create mode 100644 admin/win/nsi/lib/fileassoc.nsh diff --git a/CMakeLists.txt b/CMakeLists.txt index 7a2feb76a..d03426e43 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -171,8 +171,6 @@ if(APPLE) set( SOCKETAPI_TEAM_IDENTIFIER_PREFIX "" CACHE STRING "SocketApi prefix (including a following dot) that must match the codesign key's TeamIdentifier/Organizational Unit" ) endif() -set(OWNCLOUD_PLACEHOLDER_SUFFIX ".owncloud" CACHE STRING "Placeholder suffix (must start with a .)") - if(BUILD_CLIENT) if(APPLE) find_package(Sparkle) diff --git a/OWNCLOUD.cmake b/OWNCLOUD.cmake index a98c0787f..5b408cf23 100644 --- a/OWNCLOUD.cmake +++ b/OWNCLOUD.cmake @@ -5,6 +5,7 @@ set( APPLICATION_DOMAIN "owncloud.com" ) set( APPLICATION_VENDOR "ownCloud" ) set( APPLICATION_UPDATE_URL "https://updates.owncloud.com/client/" CACHE string "URL for updater" ) set( APPLICATION_ICON_NAME "owncloud" ) +set( APPLICATION_PLACEHOLDER_SUFFIX "owncloud" CACHE STRING "Placeholder suffix (not including the .)") set( LINUX_PACKAGE_SHORTNAME "owncloud" ) diff --git a/admin/win/nsi/lib/fileassoc.nsh b/admin/win/nsi/lib/fileassoc.nsh new file mode 100644 index 000000000..87e6caf10 --- /dev/null +++ b/admin/win/nsi/lib/fileassoc.nsh @@ -0,0 +1,120 @@ +; fileassoc.nsh +; File association helper macros +; Written by Saivert +; See http://nsis.sourceforge.net/FileAssoc +; +; Features automatic backup system and UPDATEFILEASSOC macro for +; shell change notification. +; +; |> How to use <| +; To associate a file with an application so you can double-click it in explorer, use +; the APP_ASSOCIATE macro like this: +; +; Example: +; !insertmacro APP_ASSOCIATE "txt" "myapp.textfile" "Description of txt files" \ +; "$INSTDIR\myapp.exe,0" "Open with myapp" "$INSTDIR\myapp.exe $\"%1$\"" +; +; Never insert the APP_ASSOCIATE macro multiple times, it is only ment +; to associate an application with a single file and using the +; the "open" verb as default. To add more verbs (actions) to a file +; use the APP_ASSOCIATE_ADDVERB macro. +; +; Example: +; !insertmacro APP_ASSOCIATE_ADDVERB "myapp.textfile" "edit" "Edit with myapp" \ +; "$INSTDIR\myapp.exe /edit $\"%1$\"" +; +; To have access to more options when registering the file association use the +; APP_ASSOCIATE_EX macro. Here you can specify the verb and what verb is to be the +; standard action (default verb). +; +; And finally: To remove the association from the registry use the APP_UNASSOCIATE +; macro. Here is another example just to wrap it up: +; !insertmacro APP_UNASSOCIATE "txt" "myapp.textfile" +; +; |> Note <| +; When defining your file class string always use the short form of your application title +; then a period (dot) and the type of file. This keeps the file class sort of unique. +; Examples: +; Winamp.Playlist +; NSIS.Script +; Photoshop.JPEGFile +; +; |> Tech info <| +; The registry key layout for a file association is: +; HKEY_CLASSES_ROOT +; = <"description"> +; shell +; = <"menu-item text"> +; command = <"command string"> +; + +!macro APP_ASSOCIATE EXT FILECLASS DESCRIPTION ICON COMMANDTEXT COMMAND + ; Backup the previously associated file class + ReadRegStr $R0 HKCR ".${EXT}" "" + WriteRegStr HKCR ".${EXT}" "${FILECLASS}_backup" "$R0" + + WriteRegStr HKCR ".${EXT}" "" "${FILECLASS}" + + WriteRegStr HKCR "${FILECLASS}" "" `${DESCRIPTION}` + WriteRegStr HKCR "${FILECLASS}\DefaultIcon" "" `${ICON}` + WriteRegStr HKCR "${FILECLASS}\shell" "" "open" + WriteRegStr HKCR "${FILECLASS}\shell\open" "" `${COMMANDTEXT}` + WriteRegStr HKCR "${FILECLASS}\shell\open\command" "" `${COMMAND}` +!macroend + +!macro APP_ASSOCIATE_EX EXT FILECLASS DESCRIPTION ICON VERB DEFAULTVERB SHELLNEW COMMANDTEXT COMMAND + ; Backup the previously associated file class + ReadRegStr $R0 HKCR ".${EXT}" "" + WriteRegStr HKCR ".${EXT}" "${FILECLASS}_backup" "$R0" + + WriteRegStr HKCR ".${EXT}" "" "${FILECLASS}" + StrCmp "${SHELLNEW}" "0" +2 + WriteRegStr HKCR ".${EXT}\ShellNew" "NullFile" "" + + WriteRegStr HKCR "${FILECLASS}" "" `${DESCRIPTION}` + WriteRegStr HKCR "${FILECLASS}\DefaultIcon" "" `${ICON}` + WriteRegStr HKCR "${FILECLASS}\shell" "" `${DEFAULTVERB}` + WriteRegStr HKCR "${FILECLASS}\shell\${VERB}" "" `${COMMANDTEXT}` + WriteRegStr HKCR "${FILECLASS}\shell\${VERB}\command" "" `${COMMAND}` +!macroend + +!macro APP_ASSOCIATE_ADDVERB FILECLASS VERB COMMANDTEXT COMMAND + WriteRegStr HKCR "${FILECLASS}\shell\${VERB}" "" `${COMMANDTEXT}` + WriteRegStr HKCR "${FILECLASS}\shell\${VERB}\command" "" `${COMMAND}` +!macroend + +!macro APP_ASSOCIATE_REMOVEVERB FILECLASS VERB + DeleteRegKey HKCR `${FILECLASS}\shell\${VERB}` +!macroend + + +!macro APP_UNASSOCIATE EXT FILECLASS + ; Backup the previously associated file class + ReadRegStr $R0 HKCR ".${EXT}" `${FILECLASS}_backup` + WriteRegStr HKCR ".${EXT}" "" "$R0" + + DeleteRegKey HKCR `${FILECLASS}` +!macroend + +!macro APP_ASSOCIATE_GETFILECLASS OUTPUT EXT + ReadRegStr ${OUTPUT} HKCR ".${EXT}" "" +!macroend + + +; !defines for use with SHChangeNotify +!ifdef SHCNE_ASSOCCHANGED +!undef SHCNE_ASSOCCHANGED +!endif +!define SHCNE_ASSOCCHANGED 0x08000000 +!ifdef SHCNF_FLUSH +!undef SHCNF_FLUSH +!endif +!define SHCNF_FLUSH 0x1000 + +!macro UPDATEFILEASSOC +; Using the system.dll plugin to call the SHChangeNotify Win32 API function so we +; can update the shell. + System::Call "shell32::SHChangeNotify(i,i,i,i) (${SHCNE_ASSOCCHANGED}, ${SHCNF_FLUSH}, 0, 0)" +!macroend + +;EOF diff --git a/cmake/modules/NSIS.template.in b/cmake/modules/NSIS.template.in index 967eebdf5..8a2edf965 100644 --- a/cmake/modules/NSIS.template.in +++ b/cmake/modules/NSIS.template.in @@ -7,6 +7,8 @@ !define APPLICATION_CMD_EXECUTABLE "@APPLICATION_EXECUTABLE@cmd.exe" !define APPLICATION_DOMAIN "@APPLICATION_DOMAIN@" !define APPLICATION_LICENSE "@APPLICATION_LICENSE@" +!define APPLICATION_PLACEHOLDER_SUFFIX "@APPLICATION_PLACEHOLDER_SUFFIX@" +!define APPLICATION_PLACEHOLDER_FILECLASS "@APPLICATION_EXECUTABLE@.@APPLICATION_PLACEHOLDER_SUFFIX@" !define WIN_SETUP_BITMAP_PATH "@WIN_SETUP_BITMAP_PATH@" !define CRASHREPORTER_EXECUTABLE "@CRASHREPORTER_EXECUTABLE@" @@ -100,6 +102,8 @@ ReserveFile "${NSISDIR}\Plugins\InstallOptions.dll" !include Library.nsh ;Used by the COM registration for shell extensions !include x64.nsh ;Used to determine the right arch for the shell extensions +!include ${source_path}/admin/win/nsi/lib/fileassoc.nsh + ;----------------------------------------------------------------------------- ; Memento selections stored in registry. ;----------------------------------------------------------------------------- @@ -466,6 +470,9 @@ Section "${APPLICATION_NAME}" SEC_APPLICATION ;CSync configs File "${SOURCE_PATH}/sync-exclude.lst" + ;Add file association + !insertmacro APP_ASSOCIATE "${APPLICATION_PLACEHOLDER_SUFFIX}" "${APPLICATION_PLACEHOLDER_FILECLASS}" "Placeholder for Remote File" "$INSTDIR\${APPLICATION_EXECUTABLE},0" "Download" "$INSTDIR\${APPLICATION_EXECUTABLE} $\"%1$\"" + SectionEnd !ifdef OPTION_SECTION_SC_SHELL_EXT @@ -643,6 +650,9 @@ Section Uninstall DeleteRegKey HKCR "${APPLICATION_NAME}" + ;Remove file association + !insertmacro APP_UNASSOCIATE "${APPLICATION_PLACEHOLDER_SUFFIX}" "${APPLICATION_PLACEHOLDER_FILECLASS}" + ;Shell extension !ifdef OPTION_SECTION_SC_SHELL_EXT !define LIBRARY_COM diff --git a/config.h.in b/config.h.in index 1f4372f2f..c78d52059 100644 --- a/config.h.in +++ b/config.h.in @@ -18,7 +18,8 @@ #cmakedefine APPLICATION_EXECUTABLE "@APPLICATION_EXECUTABLE@" #cmakedefine APPLICATION_UPDATE_URL "@APPLICATION_UPDATE_URL@" #cmakedefine APPLICATION_ICON_NAME "@APPLICATION_ICON_NAME@" -#cmakedefine OWNCLOUD_PLACEHOLDER_SUFFIX "@OWNCLOUD_PLACEHOLDER_SUFFIX@" +#cmakedefine APPLICATION_PLACEHOLDER_SUFFIX "@APPLICATION_PLACEHOLDER_SUFFIX@" +#define APPLICATION_DOTPLACEHOLDER_SUFFIX "." APPLICATION_PLACEHOLDER_SUFFIX #cmakedefine ZLIB_FOUND @ZLIB_FOUND@ diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 24096c2a1..34f811375 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -174,7 +174,7 @@ Application::Application(int &argc, char **argv) AbstractNetworkJob::httpTimeout = cfg.timeout(); ExcludedFiles::setupPlaceholderExclude( - cfg.excludeFile(ConfigFile::UserScope), OWNCLOUD_PLACEHOLDER_SUFFIX); + cfg.excludeFile(ConfigFile::UserScope), APPLICATION_DOTPLACEHOLDER_SUFFIX); _folderManager.reset(new FolderMan); @@ -462,7 +462,7 @@ void Application::parseOptions(const QStringList &options) _debugMode = true; } else if (option == QLatin1String("--version")) { _versionOnly = true; - } else if (option.endsWith(QStringLiteral(OWNCLOUD_PLACEHOLDER_SUFFIX))) { + } else if (option.endsWith(QStringLiteral(APPLICATION_DOTPLACEHOLDER_SUFFIX))) { // placeholder file, open it after the Folder were created (if the app is not terminated) QTimer::singleShot(0, this, [this, option] { openPlaceholder(option); }); } else { @@ -632,7 +632,7 @@ void Application::showSettingsDialog() void Application::openPlaceholder(const QString &filename) { - QString placeholderExt = QStringLiteral(OWNCLOUD_PLACEHOLDER_SUFFIX); + QString placeholderExt = QStringLiteral(APPLICATION_DOTPLACEHOLDER_SUFFIX); if (!filename.endsWith(placeholderExt)) { qWarning(lcApplication) << "Can only handle file ending in .owncloud. Unable to open" << filename; return; diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index b6f2da958..b9ed12928 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -699,7 +699,7 @@ void Folder::setSyncOptions() opt._confirmExternalStorage = cfgFile.confirmExternalStorage(); opt._moveFilesToTrash = cfgFile.moveToTrash(); opt._newFilesArePlaceholders = _definition.usePlaceholders; - opt._placeholderSuffix = QStringLiteral(OWNCLOUD_PLACEHOLDER_SUFFIX); + opt._placeholderSuffix = QStringLiteral(APPLICATION_DOTPLACEHOLDER_SUFFIX); QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE"); if (!chunkSizeEnv.isEmpty()) { diff --git a/src/gui/owncloud.xml.in b/src/gui/owncloud.xml.in index 5d1d06234..8bc9b48fe 100644 --- a/src/gui/owncloud.xml.in +++ b/src/gui/owncloud.xml.in @@ -2,6 +2,6 @@ @APPLICATION_NAME@ placeholders - + diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp index 5bacc6c49..1a663ad05 100644 --- a/src/gui/socketapi.cpp +++ b/src/gui/socketapi.cpp @@ -528,7 +528,7 @@ void SocketApi::command_OPEN_PRIVATE_LINK(const QString &localFile, SocketListen void SocketApi::command_DOWNLOAD_PLACEHOLDER(const QString &filesArg, SocketListener *) { QStringList files = filesArg.split(QLatin1Char('\x1e')); // Record Separator - auto placeholderSuffix = QStringLiteral(OWNCLOUD_PLACEHOLDER_SUFFIX); + auto placeholderSuffix = QStringLiteral(APPLICATION_DOTPLACEHOLDER_SUFFIX); for (const auto &file : files) { if (!file.endsWith(placeholderSuffix)) @@ -628,7 +628,7 @@ void SocketApi::command_GET_MENU_ITEMS(const QString &argument, OCC::SocketListe // Placeholder download action if (syncFolder) { - auto placeholderSuffix = QStringLiteral(OWNCLOUD_PLACEHOLDER_SUFFIX); + auto placeholderSuffix = QStringLiteral(APPLICATION_DOTPLACEHOLDER_SUFFIX); bool hasPlaceholderFile = false; for (const auto &file : files) { if (file.endsWith(placeholderSuffix)) -- cgit v1.2.3 From ba1309b3a4783e5a7e8901cc68694a766a3a5896 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 3 Apr 2018 17:30:17 +0200 Subject: Wizards: Add placeholder option and warning to account wizard Also add the warning dialog to the option in the folder wizard. --- src/gui/folderwizard.cpp | 15 +++++++++- src/gui/folderwizard.h | 3 ++ src/gui/owncloudsetupwizard.cpp | 1 + src/gui/wizard/owncloudadvancedsetuppage.cpp | 42 ++++++++++++++++++++++++---- src/gui/wizard/owncloudadvancedsetuppage.h | 4 +++ src/gui/wizard/owncloudadvancedsetuppage.ui | 36 ++++++++++++++++++++++++ src/gui/wizard/owncloudwizard.cpp | 28 +++++++++++++++++++ src/gui/wizard/owncloudwizard.h | 8 ++++++ 8 files changed, 130 insertions(+), 7 deletions(-) diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index 909c7e99d..2cf1e7aa0 100644 --- a/src/gui/folderwizard.cpp +++ b/src/gui/folderwizard.cpp @@ -482,7 +482,8 @@ FolderWizardSelectiveSync::FolderWizardSelectiveSync(const AccountPtr &account) QVBoxLayout *layout = new QVBoxLayout(this); _selectiveSync = new SelectiveSyncWidget(account, this); layout->addWidget(_selectiveSync); - _placeholderCheckBox = new QCheckBox(tr("Download placeholders instead of downloading the files (Experimental)")); + _placeholderCheckBox = new QCheckBox(tr("Create placeholders instead of downloading files (experimental)")); + connect(_placeholderCheckBox, &QCheckBox::clicked, this, &FolderWizardSelectiveSync::placeholderCheckboxClicked); layout->addWidget(_placeholderCheckBox); } @@ -525,6 +526,18 @@ void FolderWizardSelectiveSync::cleanupPage() QWizardPage::cleanupPage(); } +void FolderWizardSelectiveSync::placeholderCheckboxClicked() +{ + // The click has already had an effect on the box, so if it's + // checked it was newly activated. + if (_placeholderCheckBox->isChecked()) { + OwncloudWizard::askExperimentalPlaceholderFeature([this](bool enable) { + if (!enable) + _placeholderCheckBox->setChecked(false); + }); + } +} + // ==================================================================================== diff --git a/src/gui/folderwizard.h b/src/gui/folderwizard.h index 05602d84b..9e12d4749 100644 --- a/src/gui/folderwizard.h +++ b/src/gui/folderwizard.h @@ -130,6 +130,9 @@ public: virtual void initializePage() Q_DECL_OVERRIDE; virtual void cleanupPage() Q_DECL_OVERRIDE; +private slots: + void placeholderCheckboxClicked(); + private: SelectiveSyncWidget *_selectiveSync; QCheckBox *_placeholderCheckBox; diff --git a/src/gui/owncloudsetupwizard.cpp b/src/gui/owncloudsetupwizard.cpp index a1632fc3f..147a41686 100644 --- a/src/gui/owncloudsetupwizard.cpp +++ b/src/gui/owncloudsetupwizard.cpp @@ -617,6 +617,7 @@ void OwncloudSetupWizard::slotAssistantFinished(int result) folderDefinition.localPath = localFolder; folderDefinition.targetPath = FolderDefinition::prepareTargetPath(_remoteFolder); folderDefinition.ignoreHiddenFiles = folderMan->ignoreHiddenFiles(); + folderDefinition.usePlaceholders = _ocWizard->usePlaceholderSync(); if (folderMan->navigationPaneHelper().showInExplorerNavigationPane()) folderDefinition.navigationPaneClsid = QUuid::createUuid(); diff --git a/src/gui/wizard/owncloudadvancedsetuppage.cpp b/src/gui/wizard/owncloudadvancedsetuppage.cpp index 9257918dd..f97dad513 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.cpp +++ b/src/gui/wizard/owncloudadvancedsetuppage.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include "QProgressIndicator.h" @@ -59,6 +60,7 @@ OwncloudAdvancedSetupPage::OwncloudAdvancedSetupPage() connect(_ui.rSyncEverything, &QAbstractButton::clicked, this, &OwncloudAdvancedSetupPage::slotSyncEverythingClicked); connect(_ui.rSelectiveSync, &QAbstractButton::clicked, this, &OwncloudAdvancedSetupPage::slotSelectiveSyncClicked); + connect(_ui.rPlaceholderSync, &QAbstractButton::clicked, this, &OwncloudAdvancedSetupPage::slotPlaceholderSyncClicked); connect(_ui.bSelectiveSync, &QAbstractButton::clicked, this, &OwncloudAdvancedSetupPage::slotSelectiveSyncClicked); QIcon appIcon = theme->applicationIcon(); @@ -222,6 +224,11 @@ QStringList OwncloudAdvancedSetupPage::selectiveSyncBlacklist() const return _selectiveSyncBlacklist; } +bool OwncloudAdvancedSetupPage::usePlaceholderSync() const +{ + return _ui.rPlaceholderSync->isChecked(); +} + bool OwncloudAdvancedSetupPage::isConfirmBigFolderChecked() const { return _ui.rSyncEverything->isChecked() && _ui.confCheckBoxSize->isChecked(); @@ -292,9 +299,6 @@ void OwncloudAdvancedSetupPage::slotSelectFolder() void OwncloudAdvancedSetupPage::slotSelectiveSyncClicked() { - // Because clicking on it also changes it, restore it to the previous state in case the user cancelled the dialog - _ui.rSyncEverything->setChecked(_selectiveSyncBlacklist.isEmpty()); - AccountPtr acc = static_cast(wizard())->account(); SelectiveSyncDialog *dlg = new SelectiveSyncDialog(acc, _remoteFolder, _selectiveSyncBlacklist, this); @@ -317,7 +321,7 @@ void OwncloudAdvancedSetupPage::slotSelectiveSyncClicked() if (updateBlacklist) { if (!_selectiveSyncBlacklist.isEmpty()) { _ui.rSelectiveSync->blockSignals(true); - _ui.rSelectiveSync->setChecked(true); + setRadioChecked(_ui.rSelectiveSync); _ui.rSelectiveSync->blockSignals(false); auto s = dlg->estimatedSize(); if (s > 0) { @@ -326,17 +330,29 @@ void OwncloudAdvancedSetupPage::slotSelectiveSyncClicked() _ui.lSelectiveSyncSizeLabel->setText(QString()); } } else { - _ui.rSyncEverything->setChecked(true); + setRadioChecked(_ui.rSyncEverything); _ui.lSelectiveSyncSizeLabel->setText(QString()); } wizard()->setProperty("blacklist", _selectiveSyncBlacklist); } } +void OwncloudAdvancedSetupPage::slotPlaceholderSyncClicked() +{ + OwncloudWizard::askExperimentalPlaceholderFeature([this](bool enable) { + if (!enable) + return; + + _ui.lSelectiveSyncSizeLabel->setText(QString()); + _selectiveSyncBlacklist.clear(); + setRadioChecked(_ui.rPlaceholderSync); + }); +} + void OwncloudAdvancedSetupPage::slotSyncEverythingClicked() { _ui.lSelectiveSyncSizeLabel->setText(QString()); - _ui.rSyncEverything->setChecked(true); + setRadioChecked(_ui.rSyncEverything); _selectiveSyncBlacklist.clear(); } @@ -345,4 +361,18 @@ void OwncloudAdvancedSetupPage::slotQuotaRetrieved(const QVariantMap &result) _ui.lSyncEverythingSizeLabel->setText(tr("(%1)").arg(Utility::octetsToString(result["size"].toDouble()))); } +void OwncloudAdvancedSetupPage::setRadioChecked(QRadioButton *radio) +{ + // We don't want clicking the radio buttons to immediately adjust the checked state + // for selective sync and placeholder sync, so we keep them uncheckable until + // they should be checked. + radio->setCheckable(true); + radio->setChecked(true); + + if (radio != _ui.rSelectiveSync) + _ui.rSelectiveSync->setCheckable(false); + if (radio != _ui.rPlaceholderSync) + _ui.rPlaceholderSync->setCheckable(false); +} + } // namespace OCC diff --git a/src/gui/wizard/owncloudadvancedsetuppage.h b/src/gui/wizard/owncloudadvancedsetuppage.h index a51c51b1b..3db9cdffe 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.h +++ b/src/gui/wizard/owncloudadvancedsetuppage.h @@ -41,6 +41,7 @@ public: bool validatePage() Q_DECL_OVERRIDE; QString localFolder() const; QStringList selectiveSyncBlacklist() const; + bool usePlaceholderSync() const; bool isConfirmBigFolderChecked() const; void setRemoteFolder(const QString &remoteFolder); void setMultipleFoldersExist(bool exist); @@ -56,9 +57,12 @@ private slots: void slotSelectFolder(); void slotSyncEverythingClicked(); void slotSelectiveSyncClicked(); + void slotPlaceholderSyncClicked(); void slotQuotaRetrieved(const QVariantMap &result); private: + void setRadioChecked(QRadioButton *radio); + void setupCustomization(); void updateStatus(); bool dataChanged(); diff --git a/src/gui/wizard/owncloudadvancedsetuppage.ui b/src/gui/wizard/owncloudadvancedsetuppage.ui index a36704c38..77971edea 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.ui +++ b/src/gui/wizard/owncloudadvancedsetuppage.ui @@ -343,6 +343,9 @@ + + false + @@ -377,6 +380,39 @@ + + + + + + + 0 + 0 + + + + Create placeholders instead of downloading files (experimental) + + + false + + + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + + diff --git a/src/gui/wizard/owncloudwizard.cpp b/src/gui/wizard/owncloudwizard.cpp index 991b5fc18..d35838cc4 100644 --- a/src/gui/wizard/owncloudwizard.cpp +++ b/src/gui/wizard/owncloudwizard.cpp @@ -14,6 +14,7 @@ */ #include "account.h" +#include "config.h" #include "configfile.h" #include "theme.h" @@ -31,6 +32,7 @@ #include #include +#include #include @@ -113,6 +115,11 @@ QStringList OwncloudWizard::selectiveSyncBlacklist() const return _advancedSetupPage->selectiveSyncBlacklist(); } +bool OwncloudWizard::usePlaceholderSync() const +{ + return _advancedSetupPage->usePlaceholderSync(); +} + bool OwncloudWizard::isConfirmBigFolderChecked() const { return _advancedSetupPage->isConfirmBigFolderChecked(); @@ -245,4 +252,25 @@ AbstractCredentials *OwncloudWizard::getCredentials() const return 0; } +void OwncloudWizard::askExperimentalPlaceholderFeature(const std::function &callback) +{ + auto msgBox = new QMessageBox( + QMessageBox::Warning, + tr("Enable experimental feature?"), + tr("When the \"synchronize placeholders\" mode is enabled no files will be downloaded initially. " + "Instead, a tiny \"%1\" file will be created for each file on the server. " + "The contents can be downloaded by running these files or by using their context menu." + "\n\n" + "This is a new, experimental mode. If you decide to use it, please report any " + "issues that come up.") + .arg(APPLICATION_DOTPLACEHOLDER_SUFFIX)); + msgBox->addButton(tr("Enable experimental mode"), QMessageBox::AcceptRole); + msgBox->addButton(tr("Stay safe"), QMessageBox::RejectRole); + connect(msgBox, &QMessageBox::finished, msgBox, [callback, msgBox](int result) { + callback(result == QMessageBox::AcceptRole); + msgBox->deleteLater(); + }); + msgBox->open(); +} + } // end namespace diff --git a/src/gui/wizard/owncloudwizard.h b/src/gui/wizard/owncloudwizard.h index e82817a03..f51a756e9 100644 --- a/src/gui/wizard/owncloudwizard.h +++ b/src/gui/wizard/owncloudwizard.h @@ -63,6 +63,7 @@ public: QString ocUrl() const; QString localFolder() const; QStringList selectiveSyncBlacklist() const; + bool usePlaceholderSync() const; bool isConfirmBigFolderChecked() const; void enableFinishOnResultWidget(bool enable); @@ -70,6 +71,13 @@ public: void displayError(const QString &, bool retryHTTPonly); AbstractCredentials *getCredentials() const; + /** + * Shows a dialog explaining the placeholder mode and warning about it + * being experimental. Calles the callback with true if enabling was + * chosen. + */ + static void askExperimentalPlaceholderFeature(const std::function &callback); + // FIXME: Can those be local variables? // Set from the OwncloudSetupPage, later used from OwncloudHttpCredsPage QSslKey _clientSslKey; -- cgit v1.2.3 From b7412a4f0906c240b83588d7e88b965a9653bac6 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Thu, 5 Apr 2018 13:35:25 +0200 Subject: Wizards: Show placeholder option only if showExperimentalOptions is set This config file option will also control other features in the future. --- src/gui/folderwizard.cpp | 9 ++++++--- src/gui/wizard/owncloudadvancedsetuppage.cpp | 8 ++++++++ src/gui/wizard/owncloudadvancedsetuppage.ui | 4 ++-- src/libsync/configfile.cpp | 7 +++++++ src/libsync/configfile.h | 3 +++ 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index 2cf1e7aa0..6bb40f685 100644 --- a/src/gui/folderwizard.cpp +++ b/src/gui/folderwizard.cpp @@ -482,9 +482,12 @@ FolderWizardSelectiveSync::FolderWizardSelectiveSync(const AccountPtr &account) QVBoxLayout *layout = new QVBoxLayout(this); _selectiveSync = new SelectiveSyncWidget(account, this); layout->addWidget(_selectiveSync); - _placeholderCheckBox = new QCheckBox(tr("Create placeholders instead of downloading files (experimental)")); - connect(_placeholderCheckBox, &QCheckBox::clicked, this, &FolderWizardSelectiveSync::placeholderCheckboxClicked); - layout->addWidget(_placeholderCheckBox); + + if (ConfigFile().showExperimentalOptions()) { + _placeholderCheckBox = new QCheckBox(tr("Create placeholders instead of downloading files (experimental)")); + connect(_placeholderCheckBox, &QCheckBox::clicked, this, &FolderWizardSelectiveSync::placeholderCheckboxClicked); + layout->addWidget(_placeholderCheckBox); + } } FolderWizardSelectiveSync::~FolderWizardSelectiveSync() diff --git a/src/gui/wizard/owncloudadvancedsetuppage.cpp b/src/gui/wizard/owncloudadvancedsetuppage.cpp index f97dad513..2b2824558 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.cpp +++ b/src/gui/wizard/owncloudadvancedsetuppage.cpp @@ -104,6 +104,14 @@ void OwncloudAdvancedSetupPage::initializePage() { WizardCommon::initErrorLabel(_ui.errorLabel); + if (!ConfigFile().showExperimentalOptions()) { + // If the layout were wrapped in a widget, the auto-grouping of the + // radio buttons no longer works and there are surprising margins. + // Just manually hide the button and remove the layout. + _ui.rPlaceholderSync->hide(); + _ui.wSyncStrategy->layout()->removeItem(_ui.lPlaceholderSync); + } + _checking = false; _ui.lSelectiveSyncSizeLabel->setText(QString()); _ui.lSyncEverythingSizeLabel->setText(QString()); diff --git a/src/gui/wizard/owncloudadvancedsetuppage.ui b/src/gui/wizard/owncloudadvancedsetuppage.ui index 77971edea..b63c05620 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.ui +++ b/src/gui/wizard/owncloudadvancedsetuppage.ui @@ -227,7 +227,7 @@ - + 0 @@ -381,7 +381,7 @@ - + diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index b67954f2b..82c2bc5f3 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -64,6 +64,7 @@ static const char chunkSizeC[] = "chunkSize"; static const char minChunkSizeC[] = "minChunkSize"; static const char maxChunkSizeC[] = "maxChunkSize"; static const char targetChunkUploadDurationC[] = "targetChunkUploadDuration"; +static const char showExperimentalOptionsC[] = "showExperimentalOptions"; static const char proxyHostC[] = "Proxy/host"; static const char proxyTypeC[] = "Proxy/type"; @@ -736,6 +737,12 @@ void ConfigFile::setCrashReporter(bool enabled) settings.setValue(QLatin1String(crashReporterC), enabled); } +bool ConfigFile::showExperimentalOptions() const +{ + QSettings settings(configFile(), QSettings::IniFormat); + return settings.value(QLatin1String(showExperimentalOptionsC), false).toBool(); +} + QString ConfigFile::certificatePath() const { return retrieveData(QString(), QLatin1String(certPath)).toString(); diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h index c50714517..8877bb6dc 100644 --- a/src/libsync/configfile.h +++ b/src/libsync/configfile.h @@ -89,6 +89,9 @@ public: bool crashReporter() const; void setCrashReporter(bool enabled); + // Whether experimental UI options should be shown + bool showExperimentalOptions() const; + // proxy settings void setProxyType(int proxyType, const QString &host = QString(), -- cgit v1.2.3 From a9561f494b6b6259ed7fb5f0e05e23f6e423f21d Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Mon, 9 Apr 2018 10:46:52 +0200 Subject: Doc: Add showExperimentalOptions to conffile.rst --- doc/conffile.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/conffile.rst b/doc/conffile.rst index 8cef1d272..ca1ea84d7 100644 --- a/doc/conffile.rst +++ b/doc/conffile.rst @@ -49,6 +49,9 @@ Some interesting values that can be set on the configuration file are: | ``moveToTrash`` | ``false`` | If non-locally deleted files should be moved to trash instead of deleting them completely. | | | | This option only works on linux | +---------------------------------+---------------+--------------------------------------------------------------------------------------------------------+ +| ``showExperimentalOptions`` | ``false`` | Whether to show experimental options that are still undergoing testing in the user interface. | +| | | Turning this on does not enable experimental behavior on its own. It does enable user inferface options that can be used to opt in to experimental features. | ++---------------------------------+---------------+--------------------------------------------------------------------------------------------------------+ +----------------------------------------------------------------------------------------------------------------------------------------------------------+ -- cgit v1.2.3