From 49f8143f0047c78bc09082c0fbe200e0fa074365 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Wed, 23 Nov 2016 10:40:17 +0100 Subject: [PATCH] Bugfixes for sync journal name generation and usage * Use 'user' value for journal name generation * Save journal name in settings * Make owncloudcmd choose the right db --- src/cmd/cmd.cpp | 16 ++++- src/gui/accountsettings.cpp | 3 +- src/gui/folder.cpp | 43 +++++++++++- src/gui/folder.h | 27 ++++++++ src/gui/folderman.cpp | 52 ++++++++++---- src/gui/folderman.h | 5 +- src/gui/owncloudsetupwizard.cpp | 2 +- src/libsync/syncjournaldb.cpp | 118 +++++++++++++++----------------- src/libsync/syncjournaldb.h | 25 +++---- test/syncenginetestutils.h | 3 +- test/testsyncjournaldb.cpp | 3 +- 11 files changed, 189 insertions(+), 108 deletions(-) diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp index ab9338b9e..f7ad352b5 100644 --- a/src/cmd/cmd.cpp +++ b/src/cmd/cmd.cpp @@ -382,11 +382,20 @@ int main(int argc, char **argv) { QByteArray remUrl = options.target_url.toUtf8(); // Find the folder and the original owncloud url - QStringList splitted = url.path().split(account->davPath()); + QStringList splitted = url.path().split("/" + account->davPath()); url.setPath(splitted.value(0)); url.setScheme(url.scheme().replace("owncloud", "http")); - QString folder = splitted.value(1); + + QUrl credentialFreeUrl = url; + credentialFreeUrl.setUserName(QString()); + credentialFreeUrl.setPassword(QString()); + + // Remote folders typically start with a / and don't end with one + QString folder = "/" + splitted.value(1); + if (folder.endsWith("/") && folder != "/") { + folder.chop(1); + } SimpleSslErrorHandler *sslErrorHandler = new SimpleSslErrorHandler; @@ -469,7 +478,8 @@ restart_sync: } Cmd cmd; - SyncJournalDb db; + QString dbPath = options.source_dir + SyncJournalDb::makeDbName(credentialFreeUrl, folder, user); + SyncJournalDb db(dbPath); if (!selectiveSyncList.empty()) { selectiveSyncFixup(&db, selectiveSyncList); diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index c9dc99269..a5c7895f1 100644 --- a/src/gui/accountsettings.cpp +++ b/src/gui/accountsettings.cpp @@ -289,7 +289,8 @@ void AccountSettings::slotFolderWizardAccepted() FolderDefinition definition; definition.localPath = FolderDefinition::prepareLocalPath( folderWizard->field(QLatin1String("sourceFolder")).toString()); - definition.targetPath = folderWizard->property("targetPath").toString(); + definition.targetPath = FolderDefinition::prepareTargetPath( + folderWizard->property("targetPath").toString()); { QDir dir(definition.localPath); diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 756b6ff04..ff4dedb97 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -46,6 +46,8 @@ namespace OCC { +const char oldJournalPath[] = ".csync_journal.db"; + Folder::Folder(const FolderDefinition& definition, AccountState* accountState, QObject* parent) @@ -59,7 +61,9 @@ Folder::Folder(const FolderDefinition& definition, , _lastSyncDuration(0) , _consecutiveFailingSyncs(0) , _consecutiveFollowUpSyncs(0) + , _journal(_definition.absoluteJournalPath()) , _fileLog(new SyncRunFileLog) + , _saveBackwardsCompatible(false) { qRegisterMetaType("SyncFileItemVector"); qRegisterMetaType("SyncFileItem::Direction"); @@ -78,7 +82,6 @@ Folder::Folder(const FolderDefinition& definition, checkLocalPath(); _syncResult.setFolder(_definition.alias); - _journal.setAccountParameterForFilePath(path(), remoteUrl(), remotePath()); _engine.reset(new SyncEngine(_accountState->account(), path(), remotePath(), &_journal)); // pass the setting if hidden files are to be ignored, will be read in csync_update @@ -604,9 +607,9 @@ void Folder::saveToSettings() const } } - bool inFolders = _journal.mayMigrateDbLocation() || oneAccountOnly; + bool compatible = _saveBackwardsCompatible || oneAccountOnly; - if (inFolders) { + if (compatible) { settings->beginGroup(QLatin1String("Folders")); } else { settings->beginGroup(QLatin1String("Multifolders")); @@ -978,6 +981,11 @@ void Folder::scheduleThisFolderSoon() } } +void Folder::setSaveBackwardsCompatible(bool save) +{ + _saveBackwardsCompatible = save; +} + void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction, bool *cancel) { ConfigFile cfgFile; @@ -1030,6 +1038,7 @@ void FolderDefinition::save(QSettings& settings, const FolderDefinition& folder) { settings.beginGroup(FolderMan::escapeAlias(folder.alias)); settings.setValue(QLatin1String("localPath"), folder.localPath); + settings.setValue(QLatin1String("journalPath"), folder.journalPath); settings.setValue(QLatin1String("targetPath"), folder.targetPath); settings.setValue(QLatin1String("paused"), folder.paused); settings.setValue(QLatin1String("ignoreHiddenFiles"), folder.ignoreHiddenFiles); @@ -1042,6 +1051,7 @@ bool FolderDefinition::load(QSettings& settings, const QString& alias, settings.beginGroup(alias); folder->alias = FolderMan::unescapeAlias(alias); folder->localPath = settings.value(QLatin1String("localPath")).toString(); + folder->journalPath = settings.value(QLatin1String("journalPath")).toString(); folder->targetPath = settings.value(QLatin1String("targetPath")).toString(); folder->paused = settings.value(QLatin1String("paused")).toBool(); folder->ignoreHiddenFiles = settings.value(QLatin1String("ignoreHiddenFiles"), QVariant(true)).toBool(); @@ -1051,6 +1061,9 @@ bool FolderDefinition::load(QSettings& settings, const QString& alias, // code we assum /, so clean it up now. folder->localPath = prepareLocalPath(folder->localPath); + // Target paths also have a convention + folder->targetPath = prepareTargetPath(folder->targetPath); + return true; } @@ -1063,5 +1076,29 @@ QString FolderDefinition::prepareLocalPath(const QString& path) return p; } +QString FolderDefinition::prepareTargetPath(const QString &path) +{ + QString p = path; + if (p.endsWith(QLatin1Char('/'))) { + p.chop(1); + } + // Doing this second ensures the empty string or "/" come + // out as "/". + if (!p.startsWith(QLatin1Char('/'))) { + p.prepend(QLatin1Char('/')); + } + return p; +} + +QString FolderDefinition::absoluteJournalPath() const +{ + return QDir(localPath).filePath(journalPath); +} + +QString FolderDefinition::defaultJournalPath(AccountPtr account) +{ + return SyncJournalDb::makeDbName(account->url(), targetPath, account->credentials()->user()); +} + } // namespace OCC diff --git a/src/gui/folder.h b/src/gui/folder.h index 18f3eeaa5..b44f5799b 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -53,6 +53,8 @@ public: QString alias; /// path on local machine QString localPath; + /// path to the journal, usually relative to localPath + QString journalPath; /// path on remote QString targetPath; /// whether the folder is paused @@ -69,6 +71,15 @@ public: /// Ensure / as separator and trailing /. static QString prepareLocalPath(const QString& path); + + /// Ensure starting / and no ending /. + static QString prepareTargetPath(const QString& path); + + /// journalPath relative to localPath. + QString absoluteJournalPath() const; + + /// Returns the relative journal path that's appropriate for this folder and account. + QString defaultJournalPath(AccountPtr account); }; /** @@ -206,6 +217,12 @@ public: */ void scheduleThisFolderSoon(); + /** + * Migration: When this flag is true, this folder will save to + * the backwards-compatible 'Folders' section in the config file. + */ + void setSaveBackwardsCompatible(bool save); + signals: void syncStateChange(); void syncStarted(); @@ -334,6 +351,16 @@ private: QScopedPointer _fileLog; QTimer _scheduleSelfTimer; + + /** + * When the same local path is synced to multiple accounts, only one + * of them can be stored in the settings in a way that's compatible + * with old clients that don't support it. This flag marks folders + * that shall be written in a backwards-compatible way, by being set + * on the *first* Folder instance that was configured for each local + * path. + */ + bool _saveBackwardsCompatible; }; } diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 10ea5df10..1b558eb75 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -227,14 +227,27 @@ int FolderMan::setupFolders() return _folderMap.size(); } -void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account, bool mayMigrateOldDb) +void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account, bool backwardsCompatible) { foreach (const auto& folderAlias, settings.childGroups()) { FolderDefinition folderDefinition; if (FolderDefinition::load(settings, folderAlias, &folderDefinition)) { - Folder* f = addFolderInternal(std::move(folderDefinition), account.data(), false); + // Migration: Old settings don't have journalPath + if (folderDefinition.journalPath.isEmpty()) { + folderDefinition.journalPath = folderDefinition.defaultJournalPath(account->account()); + } + folderDefinition.defaultJournalPath(account->account()); + // Migration: If an old db is found, move it to the new name. + if (backwardsCompatible) { + SyncJournalDb::maybeMigrateDb(folderDefinition.localPath, folderDefinition.absoluteJournalPath()); + } + + Folder* f = addFolderInternal(std::move(folderDefinition), account.data()); if (f) { - f->journalDb()->setMayMigrateDbLocation(mayMigrateOldDb); + // Migration: Mark folders that shall be saved in a backwards-compatible way + if (backwardsCompatible) { + f->setSaveBackwardsCompatible(true); + } scheduleFolder(f); emit folderSyncStateChange(f); } @@ -413,7 +426,7 @@ Folder* FolderMan::setupFolderFromOldConfigFile(const QString &file, AccountStat folderDefinition.paused = paused; folderDefinition.ignoreHiddenFiles = ignoreHiddenFiles(); - folder = addFolderInternal(folderDefinition, accountState, false); + folder = addFolderInternal(folderDefinition, accountState); if (folder) { QStringList blackList = settings.value( QLatin1String("blackList")).toStringList(); if (!blackList.empty()) { @@ -844,7 +857,26 @@ void FolderMan::slotFolderSyncFinished( const SyncResult& ) Folder* FolderMan::addFolder(AccountState* accountState, const FolderDefinition& folderDefinition) { - auto folder = addFolderInternal(folderDefinition, accountState, true); + // Choose a db filename + auto definition = folderDefinition; + definition.journalPath = definition.defaultJournalPath(accountState->account()); + + if (!ensureJournalGone(definition.absoluteJournalPath())) { + return 0; + } + + auto folder = addFolderInternal(definition, accountState); + + // Migration: The first account that's configured for a local folder shall + // be saved in a backwards-compatible way. + bool oneAccountOnly = true; + foreach (Folder* other, FolderMan::instance()->map()) { + if (other != folder && other->cleanPath() == folder->cleanPath()) { + oneAccountOnly = false; + break; + } + } + folder->setSaveBackwardsCompatible(oneAccountOnly); if(folder) { folder->saveToSettings(); @@ -855,8 +887,7 @@ Folder* FolderMan::addFolder(AccountState* accountState, const FolderDefinition& } Folder* FolderMan::addFolderInternal(FolderDefinition folderDefinition, - AccountState* accountState, - bool wipeJournal) + AccountState* accountState) { auto alias = folderDefinition.alias; int count = 0; @@ -867,13 +898,6 @@ Folder* FolderMan::addFolderInternal(FolderDefinition folderDefinition, auto folder = new Folder(folderDefinition, accountState, this ); - if (wipeJournal) { - if (!ensureJournalGone(folder->journalDb()->databaseFilePath())) { - delete folder; - return 0; - } - } - qDebug() << "Adding folder to Folder Map " << folder << folder->alias(); _folderMap[folder->alias()] = folder; if (folder->syncPaused()) { diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 2e088c675..ad535cb92 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -258,8 +258,7 @@ private: * does not set an account on the new folder. */ Folder* addFolderInternal(FolderDefinition folderDefinition, - AccountState* accountState, - bool wipeJournal); + AccountState* accountState); /* unloads a folder object, does not delete it */ void unloadFolder( Folder * ); @@ -275,7 +274,7 @@ private: // restarts the application (Linux only) void restartApplication(); - void setupFoldersHelper(QSettings& settings, AccountStatePtr account, bool mayMigrateOldDb); + void setupFoldersHelper(QSettings& settings, AccountStatePtr account, bool backwardsCompatible); QSet _disabledFolders; Folder::Map _folderMap; diff --git a/src/gui/owncloudsetupwizard.cpp b/src/gui/owncloudsetupwizard.cpp index 993bf04f0..674b8d592 100644 --- a/src/gui/owncloudsetupwizard.cpp +++ b/src/gui/owncloudsetupwizard.cpp @@ -496,7 +496,7 @@ void OwncloudSetupWizard::slotAssistantFinished( int result ) qDebug() << "Adding folder definition for" << localFolder << _remoteFolder; FolderDefinition folderDefinition; folderDefinition.localPath = localFolder; - folderDefinition.targetPath = _remoteFolder; + folderDefinition.targetPath = FolderDefinition::prepareTargetPath(_remoteFolder); folderDefinition.ignoreHiddenFiles = folderMan->ignoreHiddenFiles(); auto f = folderMan->addFolder(account, folderDefinition); diff --git a/src/libsync/syncjournaldb.cpp b/src/libsync/syncjournaldb.cpp index 85c59cbad..43e06ba29 100644 --- a/src/libsync/syncjournaldb.cpp +++ b/src/libsync/syncjournaldb.cpp @@ -32,52 +32,72 @@ namespace OCC { -SyncJournalDb::SyncJournalDb( QObject *parent) : +SyncJournalDb::SyncJournalDb(const QString& dbFilePath, QObject *parent) : QObject(parent), - _transaction(0), - _mayMigrateDbLocation(false) + _dbFile(dbFilePath), + _transaction(0) { } -bool SyncJournalDb::exists() +QString SyncJournalDb::makeDbName(const QUrl& remoteUrl, + const QString& remotePath, + const QString& user) { - QMutexLocker locker(&_mutex); - return (!_dbFile.isEmpty() && QFile::exists(_dbFile)); -} + QString journalPath = QLatin1String("._sync_"); -void SyncJournalDb::setAccountParameterForFilePath( const QString& localPath, const QUrl& remoteUrl, const QString& remotePath ) -{ - // localPath always has a trailing slash - _dbFile = localPath; - _dbFile.append( QLatin1String("._sync_")); - // FIXME: Maybe it is better to only allow different hosts, without path component. - QString remoteUrlPath = remoteUrl.toString(); - if( remotePath != QLatin1String("/") ) { - remoteUrlPath.append(remotePath); - } - QByteArray ba = QCryptographicHash::hash( remoteUrlPath.toUtf8(), QCryptographicHash::Md5); - _dbFile.append( ba.left(6).toHex() ); - _dbFile.append(".db"); -} + QString key = QString::fromUtf8("%1@%2:%3").arg( + user, + remoteUrl.toString(), + remotePath); -bool SyncJournalDb::mayMigrateDbLocation() const -{ - return _mayMigrateDbLocation; + QByteArray ba = QCryptographicHash::hash(key.toUtf8(), QCryptographicHash::Md5); + journalPath.append( ba.left(6).toHex() ); + journalPath.append(".db"); + + return journalPath; } -void SyncJournalDb::setMayMigrateDbLocation(bool migrate) +bool SyncJournalDb::maybeMigrateDb(const QString& localPath, const QString& absoluteJournalPath) { - _mayMigrateDbLocation = migrate; + const QString oldDbName = localPath + QLatin1String(".csync_journal.db"); + if( !FileSystem::fileExists(oldDbName) ) { + return true; + } + + const QString newDbName = absoluteJournalPath; + + // Whenever there is an old db file, migrate it to the new db path. + // This is done to make switching from older versions to newer versions + // work correctly even if the user had previously used a new version + // and therefore already has an (outdated) new-style db file. + QString error; + + if( FileSystem::fileExists( newDbName ) ) { + if( !FileSystem::remove(newDbName, &error) ) { + qDebug() << "Database migration: Could not remove db file" << newDbName + << "due to" << error; + return false; + } + } + + if( !FileSystem::rename(oldDbName, newDbName, &error) ) { + qDebug() << "Database migration: could not rename " << oldDbName + << "to" << newDbName << ":" << error; + return false; + } + + qDebug() << "Journal successfully migrated from" << oldDbName << "to" << newDbName; + return true; } -#ifndef NDEBUG -void SyncJournalDb::setDatabaseFilePath( const QString& dbFile) +bool SyncJournalDb::exists() { - _dbFile = dbFile; + QMutexLocker locker(&_mutex); + return (!_dbFile.isEmpty() && QFile::exists(_dbFile)); } -#endif -QString SyncJournalDb::databaseFilePath() + +QString SyncJournalDb::databaseFilePath() const { return _dbFile; } @@ -163,35 +183,6 @@ bool SyncJournalDb::checkConnect() return false; } - const QString dir = _dbFile.left( _dbFile.lastIndexOf(QChar('/')) ); - const QString oldDbName = dir + QLatin1String("/.csync_journal.db"); - - bool migrateOldDb = _mayMigrateDbLocation && FileSystem::fileExists(oldDbName); - - // Whenever there is an old db file, migrate it to the new db path. - // This is done to make switching from older versions to newer versions - // work correctly even if the user had previously used a new version - // and therefore already has an (outdated) new-style db file. - if( migrateOldDb ) { - QString error; - - if( FileSystem::fileExists( _dbFile ) ) { - if( !FileSystem::remove(_dbFile, &error) ) { - qDebug() << "Database migration: Could not remove db file" << _dbFile - << "due to" << error; - return false; - } - } - - if( !FileSystem::rename(oldDbName, _dbFile, &error) ) { - qDebug() << "Database migration: could not rename " << oldDbName - << "to" << _dbFile << ":" << error; - return false; - } - - qDebug() << "Journal successfully migrated from" << oldDbName << "to" << _dbFile; - } - // The database file is created by this call (SQLITE_OPEN_CREATE) if( !_db.openOrCreateReadWrite(_dbFile) ) { QString error = _db.error(); @@ -365,10 +356,9 @@ bool SyncJournalDb::checkConnect() SqlQuery versionQuery("SELECT major, minor, patch FROM version;", _db); if (!versionQuery.next()) { // If there was no entry in the table, it means we are likely upgrading from 1.5 - if (migrateOldDb) { - qDebug() << Q_FUNC_INFO << "possibleUpgradeFromMirall_1_5 detected!"; - forceRemoteDiscovery = true; - } + qDebug() << Q_FUNC_INFO << "possibleUpgradeFromMirall_1_5 detected!"; + forceRemoteDiscovery = true; + createQuery.prepare("INSERT INTO version VALUES (?1, ?2, ?3, ?4);"); createQuery.bindValue(1, MIRALL_VERSION_MAJOR); createQuery.bindValue(2, MIRALL_VERSION_MINOR); diff --git a/src/libsync/syncjournaldb.h b/src/libsync/syncjournaldb.h index fe6a6602c..1f2ba6d8b 100644 --- a/src/libsync/syncjournaldb.h +++ b/src/libsync/syncjournaldb.h @@ -37,9 +37,17 @@ class OWNCLOUDSYNC_EXPORT SyncJournalDb : public QObject { Q_OBJECT public: - explicit SyncJournalDb(QObject *parent = 0); + explicit SyncJournalDb(const QString& dbFilePath, QObject *parent = 0); virtual ~SyncJournalDb(); + /// Create a journal path for a specific configuration + static QString makeDbName(const QUrl& remoteUrl, + const QString& remotePath, + const QString& user); + + /// Migrate a csync_journal to the new path, if necessary. Returns false on error + static bool maybeMigrateDb(const QString& localPath, const QString& absoluteJournalPath); + // to verify that the record could be queried successfully check // with SyncJournalFileRecord::isValid() SyncJournalFileRecord getFileRecord(const QString& filename); @@ -58,14 +66,7 @@ public: bool exists(); void walCheckpoint(); - QString databaseFilePath(); -#ifndef NDEBUG - void setDatabaseFilePath( const QString& dbFile); -#endif - void setAccountParameterForFilePath(const QString& localPath, const QUrl &remoteUrl, const QString& remotePath ); - - bool mayMigrateDbLocation() const; - void setMayMigrateDbLocation(bool migrate); + QString databaseFilePath() const; static qint64 getPHash(const QString& ); @@ -223,12 +224,6 @@ private: * that would write the etag and would void the purpose of avoidReadFromDbOnNextSync */ QList _avoidReadFromDbOnNextSyncFilter; - - /** - * Whether to check old journal path (csync_journal.db) and move the file - * to its new location, if it exists. - */ - bool _mayMigrateDbLocation; }; bool OWNCLOUDSYNC_EXPORT diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index b68f84397..88e30216c 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -737,8 +737,7 @@ public: _account->setUrl(QUrl(QStringLiteral("http://admin:admin@localhost/owncloud"))); _account->setCredentials(new FakeCredentials{_fakeQnam}); - _journalDb.reset(new OCC::SyncJournalDb()); - _journalDb->setDatabaseFilePath(localPath() + "._sync_test.db"); + _journalDb.reset(new OCC::SyncJournalDb(localPath() + "._sync_test.db")); _syncEngine.reset(new OCC::SyncEngine(_account, localPath(), "", _journalDb.get())); // A new folder will update the local file state database on first sync. diff --git a/test/testsyncjournaldb.cpp b/test/testsyncjournaldb.cpp index 19c92259a..fa6021acc 100644 --- a/test/testsyncjournaldb.cpp +++ b/test/testsyncjournaldb.cpp @@ -19,9 +19,8 @@ class TestSyncJournalDB : public QObject public: TestSyncJournalDB() + : _db("/tmp/csync-test.db") { - const QString testdb("/tmp/csync-test.db"); - _db.setDatabaseFilePath( testdb ); } QDateTime dropMsecs(QDateTime time) -- 2.30.2