From 13a2209f11677d956f8a096bde3dee10ee540418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Mon, 1 Sep 2025 02:07:36 +0200 Subject: [PATCH] SQLite improvements - (BC BREAK) Remove $WAL static property. We instead just let Laravel use its journal_mode config now - Remove journal, wal, and shm files when deleting tenant DB - Check that the system is 64-bit when using NoAttach (we don't build 32 bit extensions) - Use local static instead of a class static property for caching loadExtensionSupported --- .../SQLiteDatabaseManager.php | 40 +++++-------------- src/Features/DisallowSqliteAttach.php | 23 +++-------- tests/TenantDatabaseManagerTest.php | 38 ++++++++++-------- 3 files changed, 38 insertions(+), 63 deletions(-) diff --git a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php index 818539c2..f3181fa2 100644 --- a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Stancl\Tenancy\Database\TenantDatabaseManagers; -use AssertionError; use Closure; use Illuminate\Database\Eloquent\Model; use PDO; @@ -19,13 +18,6 @@ class SQLiteDatabaseManager implements TenantDatabaseManager */ public static string|null $path = null; - /** - * Should the WAL journal mode be used for newly created databases. - * - * @see https://www.sqlite.org/pragma.html#pragma_journal_mode - */ - public static bool $WAL = true; - /* * If this isn't null, a connection to the tenant DB will be created * and passed to the provided closure, for the purpose of keeping the @@ -89,26 +81,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager return true; } - try { - if (file_put_contents($path = $this->getPath($name), '') === false) { - return false; - } - - // todo@sqlite we can just respect Laravel config for WAL now - if (static::$WAL) { - $pdo = new PDO('sqlite:' . $path); - $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); - - // @phpstan-ignore-next-line method.nonObject - assert($pdo->query('pragma journal_mode = wal')->fetch(PDO::FETCH_ASSOC)['journal_mode'] === 'wal', 'Unable to set journal mode to wal.'); - } - - return true; - } catch (AssertionError $e) { - throw $e; - } catch (Throwable) { - return false; - } + return file_put_contents($this->getPath($name), '') !== false; } public function deleteDatabase(TenantWithDatabase $tenant): bool @@ -123,9 +96,16 @@ class SQLiteDatabaseManager implements TenantDatabaseManager return true; } + $path = $this->getPath($name); + try { - // todo@sqlite we should also remove any other files for the DB e.g. WAL - return unlink($this->getPath($name)); + unlink($path.'-journal'); + unlink($path.'-wal'); + unlink($path.'-shm'); + } catch (Throwable) {} + + try { + return unlink($path); } catch (Throwable) { return false; } diff --git a/src/Features/DisallowSqliteAttach.php b/src/Features/DisallowSqliteAttach.php index 16dd47f0..d7c57ac2 100644 --- a/src/Features/DisallowSqliteAttach.php +++ b/src/Features/DisallowSqliteAttach.php @@ -13,7 +13,6 @@ use Stancl\Tenancy\Contracts\Feature; class DisallowSqliteAttach implements Feature { - protected static bool|null $loadExtensionSupported = null; public static string|false|null $extensionPath = null; public function bootstrap(): void @@ -38,20 +37,12 @@ class DisallowSqliteAttach implements Feature protected function loadExtension(PDO $pdo): bool { - if (static::$loadExtensionSupported === null) { - // todo@sqlite refactor to local static - static::$loadExtensionSupported = method_exists($pdo, 'loadExtension'); - } + static $loadExtensionSupported = method_exists($pdo, 'loadExtension'); - if (static::$loadExtensionSupported === false) { - return false; - } - - if (static::$extensionPath === false) { - return false; - } - - // todo@sqlite we may want to check for 64 bit + if ((! $loadExtensionSupported) || + (static::$extensionPath === false) || + (PHP_INT_SIZE !== 8) + ) return false; $suffix = match (PHP_OS_FAMILY) { 'Linux' => 'so', @@ -64,9 +55,7 @@ class DisallowSqliteAttach implements Feature $arm = $arch === 'aarch64' || $arch === 'arm64'; static::$extensionPath ??= realpath(base_path('vendor/stancl/tenancy/extensions/lib/' . ($arm ? 'arm/' : '') . 'noattach.' . $suffix)); - if (static::$extensionPath === false) { - return false; - } + if (static::$extensionPath === false) return false; $pdo->loadExtension(static::$extensionPath); // @phpstan-ignore method.notFound diff --git a/tests/TenantDatabaseManagerTest.php b/tests/TenantDatabaseManagerTest.php index 051312da..a9f99829 100644 --- a/tests/TenantDatabaseManagerTest.php +++ b/tests/TenantDatabaseManagerTest.php @@ -29,6 +29,8 @@ use Stancl\Tenancy\Database\TenantDatabaseManagers\PermissionControlledPostgreSQ use Stancl\Tenancy\Database\TenantDatabaseManagers\PermissionControlledPostgreSQLDatabaseManager; use Stancl\Tenancy\Database\TenantDatabaseManagers\PermissionControlledMicrosoftSQLServerDatabaseManager; use function Stancl\Tenancy\Tests\pest; +use function Stancl\Tenancy\Tests\withBootstrapping; +use function Stancl\Tenancy\Tests\withTenantDatabases; beforeEach(function () { SQLiteDatabaseManager::$path = null; @@ -146,18 +148,15 @@ test('db name is prefixed with db path when sqlite is used', function () { expect(database_path('foodb'))->toBe(config('database.connections.tenant.database')); }); -test('sqlite databases use the WAL journal mode by default', function (bool|null $wal) { - $expected = $wal ? 'wal' : 'delete'; - if ($wal !== null) { - SQLiteDatabaseManager::$WAL = $wal; - } else { - // default behavior - $expected = 'wal'; - } - - Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { - return $event->tenant; - })->toListener()); +test('sqlite databases respect the template journal_mode config', function (string $journal_mode) { + withTenantDatabases(); + withBootstrapping(); + config([ + 'database.connections.sqlite.journal_mode' => $journal_mode, + 'tenancy.bootstrappers' => [ + DatabaseTenancyBootstrapper::class, + ], + ]); $tenant = Tenant::create([ 'tenancy_db_connection' => 'sqlite', @@ -170,11 +169,18 @@ test('sqlite databases use the WAL journal mode by default', function (bool|null $db = new PDO('sqlite:' . $dbPath); $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); - expect($db->query('pragma journal_mode')->fetch(PDO::FETCH_ASSOC)['journal_mode'])->toBe($expected); + // Before we connect to the DB using Laravel, it will be in default delete mode + expect($db->query('pragma journal_mode')->fetch(PDO::FETCH_ASSOC)['journal_mode'])->toBe('delete'); - // cleanup - SQLiteDatabaseManager::$WAL = true; -})->with([true, false, null]); + // This will trigger the logic in Laravel's SQLiteConnector + $tenant->run(fn () => DB::select('select 1')); + + $db = new PDO('sqlite:' . $dbPath); + $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + + // Once we connect to the DB, it will be in the configured journal mode + expect($db->query('pragma journal_mode')->fetch(PDO::FETCH_ASSOC)['journal_mode'])->toBe($journal_mode); +})->with(['delete', 'wal']); test('schema manager uses schema to separate tenant dbs', function () { config([