Database: Change path for new dbs to .sync_* #5904

This is to avoid issues on OSX, where the ._ prefix has special meaning. Originally (before 2.3.2) ._ was necessary to guarantee exclusion. But since then the .sync_ prefix is excluded as well. This does not affect existing database files.
owncloud · Dec 18, 2018 · 48fbee1 · 48fbee1
1 parent aa05ce4
commit 48fbee1
Show file tree

Hide file tree

Showing 9 changed files with 27 additions and 43 deletions.
diff --git a/doc/troubleshooting.rst b/doc/troubleshooting.rst
@@ -55,8 +55,8 @@ Identifying Basic Functionality Problems
 ---------------------
 
 If you see this error message stop your client, delete the
-``._sync_xxxxxxx.db`` file, and then restart your client.
-There is a  hidden ``._sync_xxxxxxx.db`` file inside the folder of every account
+``.sync_xxxxxxx.db`` file, and then restart your client.
+There is a  hidden ``.sync_xxxxxxx.db`` file inside the folder of every account
 configured on your client. 
 
 .. NOTE::

diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp
@@ -107,23 +107,14 @@ QString SyncJournalDb::makeDbName(const QString &localPath,
     const QString &remotePath,
     const QString &user)
 {
-    QString journalPath = QLatin1String("._sync_");
+    QString journalPath = QLatin1String(".sync_");
 
     QString key = QString::fromUtf8("%1@%2:%3").arg(user, remoteUrl.toString(), remotePath);
 
     QByteArray ba = QCryptographicHash::hash(key.toUtf8(), QCryptographicHash::Md5);
     journalPath.append(ba.left(6).toHex());
     journalPath.append(".db");
 
-    // If the journal doesn't exist and we can't create a file
-    // at that location, try again with a journal name that doesn't
-    // have the ._ prefix.
-    //
-    // The disadvantage of that filename is that it will only be ignored
-    // by client versions >2.3.2.
-    //
-    // See #5633: "._*" is often forbidden on samba shared folders.
-
     // If it exists already, the path is clearly usable
     QFile file(QDir(localPath).filePath(journalPath));
     if (file.exists()) {
@@ -138,19 +129,8 @@ QString SyncJournalDb::makeDbName(const QString &localPath,
         return journalPath;
     }
 
-    // Can we create it if we drop the underscore?
-    QString alternateJournalPath = journalPath.mid(2).prepend(".");
-    QFile file2(QDir(localPath).filePath(alternateJournalPath));
-    if (file2.open(QIODevice::ReadWrite)) {
-        // The alternative worked, use it
-        qCInfo(lcDb) << "Using alternate database path" << alternateJournalPath;
-        file2.close();
-        file2.remove();
-        return alternateJournalPath;
-    }
-
-    // Neither worked, just keep the original and throw errors later
-    qCWarning(lcDb) << "Could not find a writable database path" << file.fileName();
+    // Error during creation, just keep the original and throw errors later
+    qCWarning(lcDb) << "Could not find a writable database path" << file.fileName() << file.errorString();
     return journalPath;
 }
 

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
@@ -731,7 +731,7 @@ void Folder::wipe()
     // Delete files that have been partially downloaded.
     slotDiscardDownloadProgress();
 
-    //Unregister the socket API so it does not keep the ._sync_journal file open
+    //Unregister the socket API so it does not keep the .sync_journal file open
     FolderMan::instance()->socketApi()->slotUnregisterPath(alias());
     _journal.close(); // close the sync journal
 

diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp
@@ -236,12 +236,13 @@ void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account,
                 folderDefinition.journalPath = defaultJournalPath;
             }
 
-            // Migration: ._ files sometimes don't work
-            // So if the configured journalPath is the default one ("._sync_*.db")
+            // Migration: ._ files sometimes can't be created.
+            // So if the configured journalPath has a dot-underscore ("._sync_*.db")
             // but the current default doesn't have the underscore, switch to the
-            // new default. See SyncJournalDb::makeDbName().
+            // new default if no db exists yet.
             if (folderDefinition.journalPath.startsWith("._sync_")
-                && defaultJournalPath.startsWith(".sync_")) {
+                && defaultJournalPath.startsWith(".sync_")
+                && !QFile::exists(folderDefinition.absoluteJournalPath())) {
                 folderDefinition.journalPath = defaultJournalPath;
             }
 

diff --git a/test/csync/csync_tests/check_csync_exclude.cpp b/test/csync/csync_tests/check_csync_exclude.cpp
@@ -143,16 +143,17 @@ static void check_csync_excluded(void **)
     assert_int_equal(check_file_full("subdir/.csync_journal.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
 
     /* also the new form of the database name */
-    assert_int_equal(check_file_full("._sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
-    assert_int_equal(check_file_full("._sync_5bdd60bdfcfa.db.ctmp"), CSYNC_FILE_SILENTLY_EXCLUDED);
-    assert_int_equal(check_file_full("._sync_5bdd60bdfcfa.db-shm"), CSYNC_FILE_SILENTLY_EXCLUDED);
-    assert_int_equal(check_file_full("subdir/._sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
-
     assert_int_equal(check_file_full(".sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
     assert_int_equal(check_file_full(".sync_5bdd60bdfcfa.db.ctmp"), CSYNC_FILE_SILENTLY_EXCLUDED);
     assert_int_equal(check_file_full(".sync_5bdd60bdfcfa.db-shm"), CSYNC_FILE_SILENTLY_EXCLUDED);
     assert_int_equal(check_file_full("subdir/.sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
 
+    /* and the older form */
+    assert_int_equal(check_file_full("._sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
+    assert_int_equal(check_file_full("._sync_5bdd60bdfcfa.db.ctmp"), CSYNC_FILE_SILENTLY_EXCLUDED);
+    assert_int_equal(check_file_full("._sync_5bdd60bdfcfa.db-shm"), CSYNC_FILE_SILENTLY_EXCLUDED);
+    assert_int_equal(check_file_full("subdir/._sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
+
 
     /* pattern ]*.directory - ignore and remove */
     assert_int_equal(check_file_full("my.~directory"), CSYNC_FILE_EXCLUDE_AND_REMOVE);
@@ -244,15 +245,17 @@ static void check_csync_excluded_traversal(void **)
     assert_int_equal(check_file_traversal("/two/subdir/.csync_journal.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
 
     /* also the new form of the database name */
+    assert_int_equal(check_file_traversal(".sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
+    assert_int_equal(check_file_traversal(".sync_5bdd60bdfcfa.db.ctmp"), CSYNC_FILE_SILENTLY_EXCLUDED);
+    assert_int_equal(check_file_traversal(".sync_5bdd60bdfcfa.db-shm"), CSYNC_FILE_SILENTLY_EXCLUDED);
+    assert_int_equal(check_file_traversal("subdir/.sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
+
+    /* and the older form */
     assert_int_equal(check_file_traversal("._sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
     assert_int_equal(check_file_traversal("._sync_5bdd60bdfcfa.db.ctmp"), CSYNC_FILE_SILENTLY_EXCLUDED);
     assert_int_equal(check_file_traversal("._sync_5bdd60bdfcfa.db-shm"), CSYNC_FILE_SILENTLY_EXCLUDED);
     assert_int_equal(check_file_traversal("subdir/._sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
 
-    assert_int_equal(check_file_traversal(".sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
-    assert_int_equal(check_file_traversal(".sync_5bdd60bdfcfa.db.ctmp"), CSYNC_FILE_SILENTLY_EXCLUDED);
-    assert_int_equal(check_file_traversal(".sync_5bdd60bdfcfa.db-shm"), CSYNC_FILE_SILENTLY_EXCLUDED);
-    assert_int_equal(check_file_traversal("subdir/.sync_5bdd60bdfcfa.db"), CSYNC_FILE_SILENTLY_EXCLUDED);
 
     /* Other builtin excludes */
     assert_int_equal(check_file_traversal("foo/Desktop.ini"), CSYNC_NOT_EXCLUDED);

diff --git a/test/scripts/txpl/ownCloud/Test.pm b/test/scripts/txpl/ownCloud/Test.pm
@@ -455,7 +455,7 @@ sub traverse( $$;$ )
 
 	$isHere = 1 if( $acceptConflicts && !$isHere && $f =~ /conflicted copy/ );
 	$isHere = 1 if( $f =~ /\.csync/ );
-	$isHere = 1 if( $f =~ /\._sync_/ );
+	$isHere = 1 if( $f =~ /\.sync_/ );
 	assert( $isHere, "Filename local, but not remote: $f" );
     }
 

diff --git a/test/scripts/txpl/t2.pl b/test/scripts/txpl/t2.pl
@@ -176,7 +176,7 @@ ($)
 
 printInfo("simulate a owncloud 5 update by removing all the fileid");
 ## simulate a owncloud 5 update by removing all the fileid
-system( "sqlite3 " . localDir() . "._sync_*.db \"UPDATE metadata SET fileid='';\"");
+system( "sqlite3 " . localDir() . ".sync_*.db \"UPDATE metadata SET fileid='';\"");
 #refresh the ids
 csync();
 assertLocalAndRemoteDir( 'remoteToLocal1', 1);

diff --git a/test/scripts/txpl/t6.pl b/test/scripts/txpl/t6.pl
@@ -61,7 +61,7 @@ ($$)
 {
     my ($name,$num) = @_;
 
-    my $sql = "sqlite3 " . localDir() . "._sync_*.db \"SELECT md5 FROM metadata WHERE path='$name';\"";
+    my $sql = "sqlite3 " . localDir() . ".sync_*.db \"SELECT md5 FROM metadata WHERE path='$name';\"";
     open(my $fh, '-|', $sql) or die $!;
     my $etag  = <$fh>;
     close $fh;

diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h
@@ -1134,7 +1134,7 @@ class FakeFolder
         _account->setDavDisplayName("fakename");
         _account->setServerVersion("10.0.0");
 
-        _journalDb.reset(new OCC::SyncJournalDb(localPath() + "._sync_test.db"));
+        _journalDb.reset(new OCC::SyncJournalDb(localPath() + ".sync_test.db"));
         _syncEngine.reset(new OCC::SyncEngine(_account, localPath(), "", _journalDb.get()));
         // Ignore temporary files from the download. (This is in the default exclude list, but we don't load it)
         _syncEngine->excludedFiles().addManualExclude("]*.~*");