feat(ios): store databases in Application Support safely

.github/workflows/test-cpp.yml

@@ -0,0 +1,54 @@
+name: Test C++
+
+on:
+  push:
+    branches:
+      - main
+    paths:
+      - ".github/workflows/test-cpp.yml"
+      - "packages/react-native-nitro-sqlite/cpp/databaseMigration.*"
+      - "packages/react-native-nitro-sqlite/cpp/hybridObjects/HybridNitroSQLite.cpp"
+      - "packages/react-native-nitro-sqlite/cpp/sqlite/sqlite3.*"
+      - "packages/react-native-nitro-sqlite/tests/cpp/**"
+  pull_request:
+    paths:
+      - ".github/workflows/test-cpp.yml"
+      - "packages/react-native-nitro-sqlite/cpp/databaseMigration.*"
+      - "packages/react-native-nitro-sqlite/cpp/hybridObjects/HybridNitroSQLite.cpp"
+      - "packages/react-native-nitro-sqlite/cpp/sqlite/sqlite3.*"
+      - "packages/react-native-nitro-sqlite/tests/cpp/**"
+
+jobs:
+  test:
+    name: Database migration tests
+    runs-on: ubuntu-24.04
+    steps:
+      - uses: actions/checkout@v7
+
+      - name: Build bundled SQLite
+        run: |
+          clang \
+            -std=c11 \
+            -DSQLITE_THREADSAFE=2 \
+            -c packages/react-native-nitro-sqlite/cpp/sqlite/sqlite3.c \
+            -o /tmp/sqlite3.o
+
+      - name: Build migration tests
+        run: |
+          clang++ \
+            -std=c++20 \
+            -Wall \
+            -Wextra \
+            -Werror \
+            -Ipackages/react-native-nitro-sqlite/cpp \
+            -Ipackages/react-native-nitro-sqlite/cpp/sqlite \
+            packages/react-native-nitro-sqlite/cpp/databaseMigration.cpp \
+            packages/react-native-nitro-sqlite/tests/cpp/databaseMigration.test.cpp \
+            /tmp/sqlite3.o \
+            -ldl \
+            -lm \
+            -pthread \
+            -o /tmp/databaseMigrationTests
+
+      - name: Run migration tests
+        run: /tmp/databaseMigrationTests

README.md

@@ -386,7 +386,7 @@ To store databases in `Library/Application Support` instead (persistent, backed
 
 Supported values are `Documents` (the default) and `ApplicationSupport`.
 
-Databases created while the app was still using the Documents directory are automatically moved to `Library/Application Support` the first time they are opened after enabling this option, so existing users keep their data. If you later remove the option, databases already moved to `Library/Application Support` are **not** moved back.
+Databases created while the app was still using the Documents directory are automatically moved to `Library/Application Support` the first time they are opened or attached after enabling this option, so existing users keep their data. Deleting a database also removes any copy left in Documents by an interrupted migration. If you later remove the option, databases already moved to `Library/Application Support` are **not** moved back.
 
 This option has no effect when `RNNitroSQLite_AppGroup` is set, since app group databases live in the shared container.
 

packages/react-native-nitro-sqlite/cpp/databaseMigration.cpp

@@ -0,0 +1,123 @@
+#include "databaseMigration.hpp"
+#include "logs.hpp"
+#include <array>
+#include <system_error>
+
+namespace margelo::nitro::rnnitrosqlite {
+
+namespace fs = std::filesystem;
+
+namespace {
+
+  constexpr std::size_t kDatabaseFileCount = 4;
+  using DatabaseFiles = std::array<std::string, kDatabaseFileCount>;
+
+  DatabaseFiles getDatabaseFiles(const std::string& dbName);
+  bool copyDatabaseFiles(const DatabaseFiles& files, const fs::path& fromDirectory, const fs::path& toDirectory);
+  void removeAuxiliaryDatabaseFiles(const DatabaseFiles& files, const fs::path& directory);
+
+} // namespace
+
+fs::path migrateDatabase(const std::string& dbName, const fs::path& fromDirectory, const fs::path& toDirectory) {
+  const auto files = getDatabaseFiles(dbName);
+  std::error_code ec;
+  const bool sourceExists = fs::exists(fromDirectory / dbName, ec);
+
+  if (ec) {
+    LOGW("Failed to inspect database %s in its old location: %s", dbName.c_str(), ec.message().c_str());
+    return fromDirectory;
+  }
+
+  if (!sourceExists) {
+    // A completed migration may have been interrupted after deleting the database but before
+    // deleting its journals. The destination is already authoritative in that state.
+    removeAuxiliaryDatabaseFiles(files, fromDirectory);
+    return toDirectory;
+  }
+
+  // A database in the old directory is the live copy. Clear every database generation file at
+  // the destination before copying so SQLite never pairs the source with a stale journal.
+  if (!removeDatabaseFiles(dbName, toDirectory)) {
+    return fromDirectory;
+  }
+
+  fs::create_directories(toDirectory, ec);
+  if (ec) {
+    LOGW("Failed to create database migration directory %s: %s", toDirectory.string().c_str(), ec.message().c_str());
+    return fromDirectory;
+  }
+
+  if (!copyDatabaseFiles(files, fromDirectory, toDirectory)) {
+    return fromDirectory;
+  }
+
+  // Delete the database first. If this fails, every source journal must remain beside it so the
+  // caller can safely keep using the old location. Leftover journals after a successful database
+  // deletion are harmless and are removed on the next migration attempt.
+  if (!fs::remove(fromDirectory / dbName, ec) || ec) {
+    LOGW("Failed to remove migrated database %s from its old location: %s", dbName.c_str(), ec.message().c_str());
+    return fromDirectory;
+  }
+
+  removeAuxiliaryDatabaseFiles(files, fromDirectory);
+  return toDirectory;
+}
+
+bool removeDatabaseFiles(const std::string& dbName, const fs::path& directory) {
+  const auto files = getDatabaseFiles(dbName);
+
+  for (const auto& file : files) {
+    std::error_code ec;
+    fs::remove(directory / file, ec);
+    if (ec) {
+      LOGW("Failed to remove database file %s: %s", file.c_str(), ec.message().c_str());
+      return false;
+    }
+  }
+
+  return true;
+}
+
+namespace {
+
+  DatabaseFiles getDatabaseFiles(const std::string& dbName) {
+    return {dbName, dbName + "-journal", dbName + "-wal", dbName + "-shm"};
+  }
+
+  bool copyDatabaseFiles(const DatabaseFiles& files, const fs::path& fromDirectory, const fs::path& toDirectory) {
+    for (const auto& file : files) {
+      std::error_code ec;
+      const bool sourceExists = fs::exists(fromDirectory / file, ec);
+
+      if (ec) {
+        LOGW("Failed to inspect database file %s: %s", file.c_str(), ec.message().c_str());
+        return false;
+      }
+
+      if (!sourceExists) {
+        continue;
+      }
+
+      if (!fs::copy_file(fromDirectory / file, toDirectory / file, ec) || ec) {
+        LOGW("Failed to migrate database file %s: %s", file.c_str(), ec.message().c_str());
+        return false;
+      }
+    }
+
+    return true;
+  }
+
+  void removeAuxiliaryDatabaseFiles(const DatabaseFiles& files, const fs::path& directory) {
+    for (std::size_t index = 1; index < files.size(); index++) {
+      const auto& file = files[index];
+      std::error_code ec;
+      fs::remove(directory / file, ec);
+      if (ec) {
+        LOGW("Failed to remove database file %s: %s", file.c_str(), ec.message().c_str());
+      }
+    }
+  }
+
+} // namespace
+
+} // namespace margelo::nitro::rnnitrosqlite

packages/react-native-nitro-sqlite/cpp/databaseMigration.hpp

@@ -0,0 +1,13 @@
+#pragma once
+
+#include <filesystem>
+#include <string>
+
+namespace margelo::nitro::rnnitrosqlite {
+
+std::filesystem::path migrateDatabase(const std::string& dbName, const std::filesystem::path& fromDirectory,
+                                      const std::filesystem::path& toDirectory);
+
+bool removeDatabaseFiles(const std::string& dbName, const std::filesystem::path& directory);
+
+} // namespace margelo::nitro::rnnitrosqlite

packages/react-native-nitro-sqlite/cpp/hybridObjects/HybridNitroSQLite.cpp

@@ -1,6 +1,7 @@
 #include "HybridNitroSQLite.hpp"
 #include "HybridNitroSQLiteQueryResult.hpp"
 #include "NitroSQLiteException.hpp"
+#include "databaseMigration.hpp"
 #include "importSqlFile.hpp"
 #include "logs.hpp"
 #include "macros.hpp"
@@ -67,76 +68,26 @@ const std::string getDocPath(const std::optional<std::string>& location) {
   return tempDocPath;
 }
 
-// Moves a database (together with its -wal/-shm journal files) out of the directory a previous
-// app version stored it in. Committed-but-uncheckpointed writes live in the -wal file and SQLite
-// only replays it when it sits next to its database, so the set must never be separated: the
-// whole set is copied before any original is deleted, and if anything fails the intact originals
-// stay in place (the caller then keeps opening the database there) and the migration retries on
-// the next open.
-static void migrateDatabase(const std::string& dbName, const std::filesystem::path& fromDirectory,
-                            const std::filesystem::path& toDirectory) {
-  namespace fs = std::filesystem;
-  const std::string files[] = {dbName, dbName + "-wal", dbName + "-shm"};
-  std::error_code ec;
-
-  if (!fs::exists(fromDirectory / dbName, ec)) {
-    // Nothing to migrate. A previous run may have been interrupted after copying the set but
-    // before removing the journal files, so sweep any leftovers out of the old directory.
-    fs::remove(fromDirectory / (dbName + "-wal"), ec);
-    fs::remove(fromDirectory / (dbName + "-shm"), ec);
-    return;
-  }
-
-  // A database in the old directory means an older app version was writing there, so it is the
-  // live copy. Remove whatever sits at the destination (e.g. after a downgrade and re-upgrade)
-  // so a -wal from one database generation is never replayed into a database from another.
-  for (const auto& file : files) {
-    fs::remove(toDirectory / file, ec);
+const std::string getOldDocPath(const std::optional<std::string>& location) {
+  std::string oldDocPath = HybridNitroSQLite::migrationDocPath;
+  if (location) {
+    oldDocPath = oldDocPath + "/" + *location;
   }
 
-  fs::create_directories(toDirectory, ec);
-  for (const auto& file : files) {
-    if (!fs::exists(fromDirectory / file, ec)) {
-      continue;
-    }
+  return oldDocPath;
+}
 
-    if (!fs::copy_file(fromDirectory / file, toDirectory / file, ec) || ec) {
-      LOGW("Failed to migrate database file %s: %s", file.c_str(), ec.message().c_str());
-      return;
-    }
+const std::string getMigratedDocPath(const std::string& dbName, const std::optional<std::string>& location) {
+  const auto currentDocPath = getDocPath(location);
+  if (HybridNitroSQLite::migrationDocPath.empty()) {
+    return currentDocPath;
   }
 
-  // The database file is deleted first, and the journals only once that succeeds: if the
-  // database cannot be removed, the caller keeps opening it from the old directory, so its -wal
-  // must stay next to it or committed writes would be lost. An interruption after the first
-  // delete can only leave journal files behind, which the sweep above removes on the next open.
-  if (!fs::remove(fromDirectory / dbName, ec) || ec) {
-    LOGW("Failed to remove migrated database %s from its old location: %s", dbName.c_str(), ec.message().c_str());
-    return;
-  }
-  fs::remove(fromDirectory / (dbName + "-wal"), ec);
-  fs::remove(fromDirectory / (dbName + "-shm"), ec);
+  return migrateDatabase(dbName, getOldDocPath(location), currentDocPath).string();
 }
 
 void HybridNitroSQLite::open(const std::string& dbName, const std::optional<std::string>& location) {
-  auto docPath = getDocPath(location);
-
-  if (!migrationDocPath.empty()) {
-    std::string oldDocPath = migrationDocPath;
-    if (location) {
-      oldDocPath = oldDocPath + "/" + *location;
-    }
-
-    migrateDatabase(dbName, oldDocPath, docPath);
-
-    // If the database could not be moved out of its old directory, keep opening it there rather
-    // than creating a fresh empty one; the migration retries on the next open.
-    std::error_code ec;
-    if (std::filesystem::exists(std::filesystem::path(oldDocPath) / dbName, ec)) {
-      docPath = oldDocPath;
-    }
-  }
-
+  const auto docPath = getMigratedDocPath(dbName, location);
   sqliteOpenDb(dbName, docPath);
 }
 
@@ -145,18 +96,28 @@ void HybridNitroSQLite::close(const std::string& dbName) {
 };
 
 void HybridNitroSQLite::drop(const std::string& dbName, const std::optional<std::string>& location) {
-  const auto docPath = getDocPath(location);
-  sqliteRemoveDb(dbName, docPath);
+  const auto currentDocPath = getDocPath(location);
+  if (migrationDocPath.empty()) {
+    sqliteRemoveDb(dbName, currentDocPath);
+    return;
+  }
+
+  const auto oldDocPath = getOldDocPath(location);
+  std::error_code ec;
+  const bool oldDatabaseExists = std::filesystem::exists(std::filesystem::path(oldDocPath) / dbName, ec);
+  if (ec) {
+    LOGW("Failed to inspect database %s in its old location: %s", dbName.c_str(), ec.message().c_str());
+  }
+
+  sqliteRemoveDb(dbName, oldDatabaseExists || ec ? oldDocPath : currentDocPath);
+  removeDatabaseFiles(dbName, oldDocPath);
+  removeDatabaseFiles(dbName, currentDocPath);
 };
 
 void HybridNitroSQLite::attach(const std::string& mainDbName, const std::string& dbNameToAttach, const std::string& alias,
                                const std::optional<std::string>& location) {
-  std::string tempDocPath = std::string(docPath);
-  if (location) {
-    tempDocPath = tempDocPath + "/" + *location;
-  }
-
-  sqliteAttachDb(mainDbName, tempDocPath, dbNameToAttach, alias);
+  const auto attachedDocPath = getMigratedDocPath(dbNameToAttach, location);
+  sqliteAttachDb(mainDbName, attachedDocPath, dbNameToAttach, alias);
 };
 
 void HybridNitroSQLite::detach(const std::string& mainDbName, const std::string& alias) {

packages/react-native-nitro-sqlite/cpp/hybridObjects/HybridNitroSQLite.hpp

@@ -16,7 +16,7 @@ class HybridNitroSQLite : public HybridNitroSQLiteSpec {
   static std::string docPath;
   // Directory databases were stored in by previous app versions, when the platform layer has
   // relocated docPath (e.g. iOS with RNNitroSQLite_DatabaseLocation set to "ApplicationSupport").
-  // When non-empty, each database found there is moved to docPath as it is opened.
+  // When non-empty, databases found there are resolved as they are opened, attached, or dropped.
   static std::string migrationDocPath;
 
 public: