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

@@ -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);

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);

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>("SyncFileItemVector");
     qRegisterMetaType<SyncFileItem::Direction>("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
 

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<SyncRunFileLog> _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;
 };
 
 }

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()) {

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<Folder*>  _disabledFolders;
     Folder::Map    _folderMap;

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);

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);

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<QString> _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

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.

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)